codeant-ai-for-open-source[bot] commented on code in PR #40972:
URL: https://github.com/apache/superset/pull/40972#discussion_r3442732957
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1736,6 +1759,65 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
# giving LLMs enough context to construct valid configs.
+# Superset viz_type values that LLM clients routinely send where this API
+# expects its chart_type discriminator. Each maps to (chart_type, kind);
+# kind is only meaningful for the xy family.
+_VIZ_TYPE_TO_CHART_TYPE: dict[str, tuple[str, str | None]] = {
+ "bar": ("xy", "bar"),
+ "dist_bar": ("xy", "bar"),
+ "echarts_timeseries_bar": ("xy", "bar"),
+ "line": ("xy", "line"),
+ "echarts_timeseries_line": ("xy", "line"),
+ "echarts_timeseries_smooth": ("xy", "line"),
+ "area": ("xy", "area"),
+ "echarts_area": ("xy", "area"),
+ "scatter": ("xy", "scatter"),
+ "echarts_timeseries_scatter": ("xy", "scatter"),
+ "big_number_total": ("big_number", None),
+ "pivot_table_v2": ("pivot_table", None),
+}
+
+
+def _normalize_chart_request_input(data: Any) -> Any:
+ """Accept common Superset REST/form_data vocabulary in chart requests.
+
+ LLM clients reliably reach for Superset's public field names —
+ ``datasource_id``, ``viz_type``, and concrete viz plugin names such as
+ ``echarts_timeseries_bar`` — before consulting this API's schema. Each
+ rejection costs the client a model round trip, so the unambiguous
+ synonyms are translated instead of refused.
+ """
+ if not isinstance(data, dict):
+ return data
+ if "dataset_id" not in data and "datasource_id" in data:
+ data["dataset_id"] = data.pop("datasource_id")
+ config = data.get("config")
+ if isinstance(config, dict):
+ if "viz_type" in config:
+ viz_type = config["viz_type"]
+ if "chart_type" not in config:
+ config["chart_type"] = viz_type
+ config.pop("viz_type")
+ elif config.get("chart_type") != "table":
+ config.pop("viz_type")
+ chart_type = config.get("chart_type")
+ if isinstance(chart_type, str) and chart_type in
_VIZ_TYPE_TO_CHART_TYPE:
+ mapped_type, kind = _VIZ_TYPE_TO_CHART_TYPE[chart_type]
+ config["chart_type"] = mapped_type
+ if kind is not None:
+ config.setdefault("kind", kind)
+ return data
+
+
+class ChartRequestNormalizerMixin(BaseModel):
+ """Mixin translating Superset-vocabulary synonyms before validation."""
+
+ @model_validator(mode="before")
+ @classmethod
+ def _normalize_request_vocabulary(cls, data: Any) -> Any:
+ return _normalize_chart_request_input(data)
Review Comment:
**Suggestion:** Add a short docstring to this newly added validator method
to document its intent and keep new code consistently self-documented.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added method on the newly introduced
ChartRequestNormalizerMixin, and it has no docstring. The custom rule
explicitly requires new Python functions and classes to be documented inline,
so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8babf70c45af407d8b3071c6690e926b&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=8babf70c45af407d8b3071c6690e926b&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:** superset/mcp_service/chart/schemas.py
**Line:** 1815:1818
**Comment:**
*Custom Rule: Add a short docstring to this newly added validator
method to document its intent and keep new code consistently self-documented.
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%2F40972&comment_hash=373f134093fdac417c0c43dcb832539a274693e4ab65310d34dd4e734edd76ce&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=373f134093fdac417c0c43dcb832539a274693e4ab65310d34dd4e734edd76ce&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py:
##########
@@ -0,0 +1,226 @@
+# 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.
+
+"""
+Unit tests for Superset-vocabulary coercion in MCP chart request schemas.
+
+LLM clients reliably send Superset's public field names (datasource_id,
+viz_type, show_legend, plugin viz names) before consulting the MCP tool
+schema. These tests pin the translations that absorb those inputs instead
+of rejecting them.
+"""
+
+import pytest
+from pydantic import ValidationError
+
+from superset.mcp_service.chart.schemas import (
+ GenerateChartRequest,
+ GenerateExploreLinkRequest,
+ TableChartConfig,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ XYChartConfig,
+)
+from superset.mcp_service.dataset.tool.query_dataset import _validate_names
+
+XY_BAR_CONFIG = {
+ "chart_type": "xy",
+ "kind": "bar",
+ "x": {"name": "genre"},
+ "y": [{"name": "global_sales", "aggregate": "SUM"}],
+}
+
+
+class TestDatasourceIdAlias:
Review Comment:
**Suggestion:** Add a short class-level docstring describing the datasource
alias behavior covered by this test group. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python class with no docstring. The custom rule
requires newly added Python functions and classes to include docstrings, so the
suggestion correctly identifies a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cc39535b94d84d1380c68c539a0d051a&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=cc39535b94d84d1380c68c539a0d051a&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/chart/test_chart_config_coercion.py
**Line:** 48:48
**Comment:**
*Custom Rule: Add a short class-level docstring describing the
datasource alias behavior covered by this test group.
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%2F40972&comment_hash=3fec441da845f5b700d4f3ea0e0ce95c66106eb36ad1cdb4d342c2fcc6acd9ae&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=3fec441da845f5b700d4f3ea0e0ce95c66106eb36ad1cdb4d342c2fcc6acd9ae&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py:
##########
@@ -0,0 +1,226 @@
+# 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.
+
+"""
+Unit tests for Superset-vocabulary coercion in MCP chart request schemas.
+
+LLM clients reliably send Superset's public field names (datasource_id,
+viz_type, show_legend, plugin viz names) before consulting the MCP tool
+schema. These tests pin the translations that absorb those inputs instead
+of rejecting them.
+"""
+
+import pytest
+from pydantic import ValidationError
+
+from superset.mcp_service.chart.schemas import (
+ GenerateChartRequest,
+ GenerateExploreLinkRequest,
+ TableChartConfig,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ XYChartConfig,
+)
+from superset.mcp_service.dataset.tool.query_dataset import _validate_names
+
+XY_BAR_CONFIG = {
+ "chart_type": "xy",
+ "kind": "bar",
+ "x": {"name": "genre"},
+ "y": [{"name": "global_sales", "aggregate": "SUM"}],
+}
+
+
+class TestDatasourceIdAlias:
+ def test_generate_chart_accepts_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"datasource_id": 23, "config": dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 23
+
+ def test_dataset_id_wins_over_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 7, "datasource_id": 23, "config":
dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 7
+
+
+class TestVizTypeTranslation:
+ @pytest.mark.parametrize(
+ "viz_type,expected_kind",
+ [
+ ("bar", "bar"),
+ ("dist_bar", "bar"),
+ ("echarts_timeseries_bar", "bar"),
+ ("echarts_timeseries_line", "line"),
+ ("area", "area"),
+ ("echarts_timeseries_scatter", "scatter"),
+ ],
+ )
+ def test_xy_family_viz_types_map_to_xy(
+ self, viz_type: str, expected_kind: str
+ ) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["kind"]
+ config["chart_type"] = viz_type
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == expected_kind
+
+ def test_viz_type_key_is_accepted_as_chart_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["chart_type"]
+ del config["kind"]
+ config["viz_type"] = "bar"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == "bar"
+
+ def test_explicit_chart_type_wins_over_viz_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["viz_type"] = "pie"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+
+ def test_table_viz_type_is_preserved_with_explicit_chart_type(self) ->
None:
+ request = GenerateExploreLinkRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "table",
+ "viz_type": "ag-grid-table",
+ "columns": [{"name": "genre"}],
+ },
+ }
+ )
+ assert isinstance(request.config, TableChartConfig)
+ assert request.config.viz_type == "ag-grid-table"
+
+ def test_explicit_kind_is_preserved(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["chart_type"] = "echarts_timeseries_bar"
+ config["kind"] = "line"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.kind == "line"
+
+ def test_big_number_total_maps_to_big_number(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "big_number_total",
+ "metric": {"name": "global_sales", "aggregate": "SUM"},
+ },
+ }
+ )
+ assert request.config.chart_type == "big_number"
+
+
+class TestRequestModelsShareNormalization:
Review Comment:
**Suggestion:** Add a concise docstring to this class to describe
request-model normalization coverage. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new class without a docstring, which violates the rule that newly
added Python classes must include docstrings.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ef17962fb04a4d67943a7b00ca198553&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=ef17962fb04a4d67943a7b00ca198553&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/chart/test_chart_config_coercion.py
**Line:** 141:141
**Comment:**
*Custom Rule: Add a concise docstring to this class to describe
request-model normalization 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%2F40972&comment_hash=e2880e7b91fdd9fcdeb306afea80912a201fce6a02234483501a82e204fe0cb9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=e2880e7b91fdd9fcdeb306afea80912a201fce6a02234483501a82e204fe0cb9&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py:
##########
@@ -0,0 +1,226 @@
+# 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.
+
+"""
+Unit tests for Superset-vocabulary coercion in MCP chart request schemas.
+
+LLM clients reliably send Superset's public field names (datasource_id,
+viz_type, show_legend, plugin viz names) before consulting the MCP tool
+schema. These tests pin the translations that absorb those inputs instead
+of rejecting them.
+"""
+
+import pytest
+from pydantic import ValidationError
+
+from superset.mcp_service.chart.schemas import (
+ GenerateChartRequest,
+ GenerateExploreLinkRequest,
+ TableChartConfig,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ XYChartConfig,
+)
+from superset.mcp_service.dataset.tool.query_dataset import _validate_names
+
+XY_BAR_CONFIG = {
+ "chart_type": "xy",
+ "kind": "bar",
+ "x": {"name": "genre"},
+ "y": [{"name": "global_sales", "aggregate": "SUM"}],
+}
+
+
+class TestDatasourceIdAlias:
+ def test_generate_chart_accepts_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"datasource_id": 23, "config": dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 23
+
+ def test_dataset_id_wins_over_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 7, "datasource_id": 23, "config":
dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 7
+
+
+class TestVizTypeTranslation:
Review Comment:
**Suggestion:** Add a class docstring that explains the viz-type translation
scenarios validated in this test suite. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added class does not include a docstring. Since the rule requires
new Python classes to be documented inline, the suggestion reflects a real
violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fc5af245e0b84fbfa1584d44ed6ac34c&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=fc5af245e0b84fbfa1584d44ed6ac34c&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/chart/test_chart_config_coercion.py
**Line:** 62:62
**Comment:**
*Custom Rule: Add a class docstring that explains the viz-type
translation scenarios validated in this test suite.
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%2F40972&comment_hash=89f05d6f0ed5317c6bbcc964024e2f2a787c27bfba93da70a36838b388bf64d3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=89f05d6f0ed5317c6bbcc964024e2f2a787c27bfba93da70a36838b388bf64d3&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py:
##########
@@ -0,0 +1,226 @@
+# 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.
+
+"""
+Unit tests for Superset-vocabulary coercion in MCP chart request schemas.
+
+LLM clients reliably send Superset's public field names (datasource_id,
+viz_type, show_legend, plugin viz names) before consulting the MCP tool
+schema. These tests pin the translations that absorb those inputs instead
+of rejecting them.
+"""
+
+import pytest
+from pydantic import ValidationError
+
+from superset.mcp_service.chart.schemas import (
+ GenerateChartRequest,
+ GenerateExploreLinkRequest,
+ TableChartConfig,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ XYChartConfig,
+)
+from superset.mcp_service.dataset.tool.query_dataset import _validate_names
+
+XY_BAR_CONFIG = {
+ "chart_type": "xy",
+ "kind": "bar",
+ "x": {"name": "genre"},
+ "y": [{"name": "global_sales", "aggregate": "SUM"}],
+}
+
+
+class TestDatasourceIdAlias:
+ def test_generate_chart_accepts_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"datasource_id": 23, "config": dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 23
+
+ def test_dataset_id_wins_over_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 7, "datasource_id": 23, "config":
dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 7
+
+
+class TestVizTypeTranslation:
+ @pytest.mark.parametrize(
+ "viz_type,expected_kind",
+ [
+ ("bar", "bar"),
+ ("dist_bar", "bar"),
+ ("echarts_timeseries_bar", "bar"),
+ ("echarts_timeseries_line", "line"),
+ ("area", "area"),
+ ("echarts_timeseries_scatter", "scatter"),
+ ],
+ )
+ def test_xy_family_viz_types_map_to_xy(
+ self, viz_type: str, expected_kind: str
+ ) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["kind"]
+ config["chart_type"] = viz_type
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == expected_kind
+
+ def test_viz_type_key_is_accepted_as_chart_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["chart_type"]
+ del config["kind"]
+ config["viz_type"] = "bar"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == "bar"
+
+ def test_explicit_chart_type_wins_over_viz_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["viz_type"] = "pie"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+
+ def test_table_viz_type_is_preserved_with_explicit_chart_type(self) ->
None:
+ request = GenerateExploreLinkRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "table",
+ "viz_type": "ag-grid-table",
+ "columns": [{"name": "genre"}],
+ },
+ }
+ )
+ assert isinstance(request.config, TableChartConfig)
+ assert request.config.viz_type == "ag-grid-table"
+
+ def test_explicit_kind_is_preserved(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["chart_type"] = "echarts_timeseries_bar"
+ config["kind"] = "line"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.kind == "line"
+
+ def test_big_number_total_maps_to_big_number(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "big_number_total",
+ "metric": {"name": "global_sales", "aggregate": "SUM"},
+ },
+ }
+ )
+ assert request.config.chart_type == "big_number"
+
+
+class TestRequestModelsShareNormalization:
+ def test_update_chart_request(self) -> None:
+ request = UpdateChartRequest.model_validate(
+ {"identifier": 409, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.config is not None
+ assert request.config.chart_type == "xy"
+
+ def test_update_chart_preview_request(self) -> None:
+ request = UpdateChartPreviewRequest.model_validate(
+ {"datasource_id": 23, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.dataset_id == 23
+ assert request.config.chart_type == "xy"
+
+ def test_generate_explore_link_request(self) -> None:
+ request = GenerateExploreLinkRequest.model_validate(
+ {"datasource_id": 23, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.dataset_id == 23
+ assert request.config is not None
+ assert request.config.chart_type == "xy"
+
+
+class TestXYFieldCoercion:
Review Comment:
**Suggestion:** Add a docstring to this class summarizing the field-coercion
behaviors being tested. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The class is newly introduced and lacks a docstring. That matches the custom
rule requiring docstrings for new Python classes.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=78f31752a8534a09a26bff32e43b64b5&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=78f31752a8534a09a26bff32e43b64b5&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/chart/test_chart_config_coercion.py
**Line:** 165:165
**Comment:**
*Custom Rule: Add a docstring to this class summarizing the
field-coercion behaviors being tested.
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%2F40972&comment_hash=a8eec228fbc7c4353125b95751715b09cd7d6720b8817d96018b3f3cecacb12f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=a8eec228fbc7c4353125b95751715b09cd7d6720b8817d96018b3f3cecacb12f&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py:
##########
@@ -0,0 +1,226 @@
+# 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.
+
+"""
+Unit tests for Superset-vocabulary coercion in MCP chart request schemas.
+
+LLM clients reliably send Superset's public field names (datasource_id,
+viz_type, show_legend, plugin viz names) before consulting the MCP tool
+schema. These tests pin the translations that absorb those inputs instead
+of rejecting them.
+"""
+
+import pytest
+from pydantic import ValidationError
+
+from superset.mcp_service.chart.schemas import (
+ GenerateChartRequest,
+ GenerateExploreLinkRequest,
+ TableChartConfig,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ XYChartConfig,
+)
+from superset.mcp_service.dataset.tool.query_dataset import _validate_names
+
+XY_BAR_CONFIG = {
+ "chart_type": "xy",
+ "kind": "bar",
+ "x": {"name": "genre"},
+ "y": [{"name": "global_sales", "aggregate": "SUM"}],
+}
+
+
+class TestDatasourceIdAlias:
+ def test_generate_chart_accepts_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"datasource_id": 23, "config": dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 23
+
+ def test_dataset_id_wins_over_datasource_id(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 7, "datasource_id": 23, "config":
dict(XY_BAR_CONFIG)}
+ )
+ assert request.dataset_id == 7
+
+
+class TestVizTypeTranslation:
+ @pytest.mark.parametrize(
+ "viz_type,expected_kind",
+ [
+ ("bar", "bar"),
+ ("dist_bar", "bar"),
+ ("echarts_timeseries_bar", "bar"),
+ ("echarts_timeseries_line", "line"),
+ ("area", "area"),
+ ("echarts_timeseries_scatter", "scatter"),
+ ],
+ )
+ def test_xy_family_viz_types_map_to_xy(
+ self, viz_type: str, expected_kind: str
+ ) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["kind"]
+ config["chart_type"] = viz_type
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == expected_kind
+
+ def test_viz_type_key_is_accepted_as_chart_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ del config["chart_type"]
+ del config["kind"]
+ config["viz_type"] = "bar"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+ assert request.config.kind == "bar"
+
+ def test_explicit_chart_type_wins_over_viz_type(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["viz_type"] = "pie"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.chart_type == "xy"
+
+ def test_table_viz_type_is_preserved_with_explicit_chart_type(self) ->
None:
+ request = GenerateExploreLinkRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "table",
+ "viz_type": "ag-grid-table",
+ "columns": [{"name": "genre"}],
+ },
+ }
+ )
+ assert isinstance(request.config, TableChartConfig)
+ assert request.config.viz_type == "ag-grid-table"
+
+ def test_explicit_kind_is_preserved(self) -> None:
+ config = dict(XY_BAR_CONFIG)
+ config["chart_type"] = "echarts_timeseries_bar"
+ config["kind"] = "line"
+ request = GenerateChartRequest.model_validate(
+ {"dataset_id": 23, "config": config}
+ )
+ assert request.config.kind == "line"
+
+ def test_big_number_total_maps_to_big_number(self) -> None:
+ request = GenerateChartRequest.model_validate(
+ {
+ "dataset_id": 23,
+ "config": {
+ "chart_type": "big_number_total",
+ "metric": {"name": "global_sales", "aggregate": "SUM"},
+ },
+ }
+ )
+ assert request.config.chart_type == "big_number"
+
+
+class TestRequestModelsShareNormalization:
+ def test_update_chart_request(self) -> None:
+ request = UpdateChartRequest.model_validate(
+ {"identifier": 409, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.config is not None
+ assert request.config.chart_type == "xy"
+
+ def test_update_chart_preview_request(self) -> None:
+ request = UpdateChartPreviewRequest.model_validate(
+ {"datasource_id": 23, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.dataset_id == 23
+ assert request.config.chart_type == "xy"
+
+ def test_generate_explore_link_request(self) -> None:
+ request = GenerateExploreLinkRequest.model_validate(
+ {"datasource_id": 23, "config": {**XY_BAR_CONFIG, "chart_type":
"bar"}}
+ )
+ assert request.dataset_id == 23
+ assert request.config is not None
+ assert request.config.chart_type == "xy"
+
+
+class TestXYFieldCoercion:
+ def test_x_accepts_bare_column_name(self) -> None:
+ config = XYChartConfig.model_validate({**XY_BAR_CONFIG, "x": "genre"})
+ assert config.x is not None
+ assert config.x.name == "genre"
+
+ def test_y_accepts_bare_metric_names(self) -> None:
+ config = XYChartConfig.model_validate({**XY_BAR_CONFIG, "y":
["count"]})
+ assert config.y[0].name == "count"
+
+ def test_show_legend_bool_becomes_legend_config(self) -> None:
+ config = XYChartConfig.model_validate({**XY_BAR_CONFIG, "show_legend":
True})
+ assert config.legend is not None
+ assert config.legend.show is True
+
+ def test_show_legend_false(self) -> None:
+ config = XYChartConfig.model_validate({**XY_BAR_CONFIG, "show_legend":
False})
+ assert config.legend is not None
+ assert config.legend.show is False
+
+ def test_chart_orientation_alias(self) -> None:
+ config = XYChartConfig.model_validate(
+ {**XY_BAR_CONFIG, "chart_orientation": "horizontal"}
+ )
+ assert config.orientation == "horizontal"
+
+ def test_unknown_fields_still_rejected_with_suggestion(self) -> None:
+ with pytest.raises(ValidationError, match="order_desc"):
+ XYChartConfig.model_validate({**XY_BAR_CONFIG, "order_desc": True})
+
+
+class TestQueryDatasetMetricGuidance:
Review Comment:
**Suggestion:** Add a class docstring that describes the metric validation
guidance expectations covered here. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added class has no docstring, so it violates the rule that new
Python classes should be documented inline.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f8ff6edee9f54fef9114a52d8462e31b&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=f8ff6edee9f54fef9114a52d8462e31b&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/chart/test_chart_config_coercion.py
**Line:** 196:196
**Comment:**
*Custom Rule: Add a class docstring that describes the metric
validation guidance expectations covered here.
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%2F40972&comment_hash=905b8b856182715d5bb0a835989ba5afb551082bf3be959e188153f1ebb2c0b1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40972&comment_hash=905b8b856182715d5bb0a835989ba5afb551082bf3be959e188153f1ebb2c0b1&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]
