From e2d53641188a8f3f3b8368ae6e9ca3ce64f462a2 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Wed, 13 May 2026 22:12:08 +0000 Subject: [PATCH] fix(mcp): fix E501 in update_chart.py and update_chart test mocks for column validation Split an 89-char comment line and an over-limit condition in update_chart.py to satisfy the ruff E501 rule. Also applied ruff format. Two TestUpdateChartValidationGate tests expected CHART_VALIDATION_FAILED but received CHART_DATASET_NOT_FOUND because _validate_update_against_dataset calls DatasetValidator.validate_against_dataset before validate_and_compile, and the existing mocks provided a Mock() object for chart.datasource whose .id attribute is an auto-generated MagicMock (not a real int). Added a patch for DatasetValidator.validate_against_dataset returning (True, None) so the column-validation tier is bypassed and the test reaches the mocked validate_and_compile response as intended. Co-Authored-By: Claude Sonnet 4.6 --- superset/mcp_service/chart/tool/update_chart.py | 7 +++++-- .../mcp_service/chart/tool/test_update_chart.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) mode change 100644 => 100755 tests/unit_tests/mcp_service/chart/tool/test_update_chart.py diff --git a/superset/mcp_service/chart/tool/update_chart.py b/superset/mcp_service/chart/tool/update_chart.py index 88fc5e28849..caefae1b9d0 100755 --- a/superset/mcp_service/chart/tool/update_chart.py +++ b/superset/mcp_service/chart/tool/update_chart.py @@ -196,7 +196,8 @@ def _validate_update_against_dataset( } ) - # Column existence + fuzzy-match validation (mirrors generate_chart pipeline layer 2) + # Column existence + fuzzy-match validation + # (mirrors generate_chart pipeline layer 2) from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator is_col_valid, col_error = DatasetValidator.validate_against_dataset( @@ -414,11 +415,13 @@ async def update_chart( # noqa: C901 # Normalize column case to match dataset canonical names # (mirrors generate_chart pipeline layer 4) - if parsed_config is not None and getattr(chart, "datasource_id", None) is not None: + chart_datasource_id = getattr(chart, "datasource_id", None) + if parsed_config is not None and chart_datasource_id is not None: from superset.mcp_service.chart.validation.dataset_validator import ( DatasetValidator, NORMALIZATION_EXCEPTIONS, ) + try: parsed_config = DatasetValidator.normalize_column_names( parsed_config, chart.datasource_id diff --git a/tests/unit_tests/mcp_service/chart/tool/test_update_chart.py b/tests/unit_tests/mcp_service/chart/tool/test_update_chart.py old mode 100644 new mode 100755 index c504d8bca59..21e2286fb6f --- a/tests/unit_tests/mcp_service/chart/tool/test_update_chart.py +++ b/tests/unit_tests/mcp_service/chart/tool/test_update_chart.py @@ -1175,6 +1175,11 @@ class TestUpdateChartValidationGate: ) @patch("superset.daos.chart.ChartDAO.find_by_id", new_callable=Mock) @patch("superset.db.session") + @patch( + "superset.mcp_service.chart.validation.dataset_validator" + ".DatasetValidator.validate_against_dataset", + new=Mock(return_value=(True, None)), + ) @pytest.mark.asyncio async def test_preview_path_validation_failure_skips_cache( self, @@ -1238,6 +1243,11 @@ class TestUpdateChartValidationGate: ) @patch("superset.daos.chart.ChartDAO.find_by_id", new_callable=Mock) @patch("superset.db.session") + @patch( + "superset.mcp_service.chart.validation.dataset_validator" + ".DatasetValidator.validate_against_dataset", + new=Mock(return_value=(True, None)), + ) @pytest.mark.asyncio async def test_persist_path_validation_failure_skips_db_write( self,