From 3ebcf86d1cc6dce988917c4e70515a4e07319c49 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Wed, 10 Jun 2026 23:16:36 +0000 Subject: [PATCH] feat(mcp): warn on native_viz_types collision at plugin registration If two plugins claim the same viz_type, display_name_for_viz_type() silently resolves to the iteration-order winner. Surface a warning at register() time so plugin authors catch the shadowing immediately. --- superset/mcp_service/chart/registry.py | 15 ++++++++++++ .../mcp_service/chart/test_registry.py | 23 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/superset/mcp_service/chart/registry.py b/superset/mcp_service/chart/registry.py index 0c50a84589b..c2db7240d6a 100644 --- a/superset/mcp_service/chart/registry.py +++ b/superset/mcp_service/chart/registry.py @@ -160,6 +160,21 @@ def register(plugin: "ChartTypePlugin") -> None: logger.warning( "Overwriting existing plugin for chart_type=%r", plugin.chart_type ) + for existing in _REGISTRY.values(): + if existing.chart_type == plugin.chart_type: + continue + colliding = plugin.native_viz_types.keys() & existing.native_viz_types + if colliding: + # display_name_for_viz_type() resolves to the first plugin in + # iteration order, making the later registration unreachable. + logger.warning( + "Plugin %r declares native_viz_types %s already claimed by " + "plugin %r; viz_type display-name lookups will resolve to " + "the earlier registration", + plugin.chart_type, + sorted(colliding), + existing.chart_type, + ) _REGISTRY[plugin.chart_type] = plugin logger.debug("Registered chart plugin: %r", plugin.chart_type) diff --git a/tests/unit_tests/mcp_service/chart/test_registry.py b/tests/unit_tests/mcp_service/chart/test_registry.py index 105eaed2aa8..b455a240a54 100644 --- a/tests/unit_tests/mcp_service/chart/test_registry.py +++ b/tests/unit_tests/mcp_service/chart/test_registry.py @@ -98,6 +98,29 @@ def test_register_warns_on_duplicate(caplog): assert "Overwriting" in caplog.text +def test_register_warns_on_viz_type_collision(caplog): + register(_FakePlugin()) + + class _CollidingPlugin(BaseChartPlugin): + chart_type = "colliding" + display_name = "Colliding Chart" + native_viz_types = {"fake_viz": "Shadowed Viz", "own_viz": "Own Viz"} + + with caplog.at_level("WARNING"): + register(_CollidingPlugin()) + assert "already claimed by" in caplog.text + assert "fake_viz" in caplog.text + # Earlier registration wins in display-name lookups + assert display_name_for_viz_type("fake_viz") == "Fake Viz" + + +def test_register_same_plugin_no_collision_warning(caplog): + register(_FakePlugin()) + with caplog.at_level("WARNING"): + register(_FakePlugin()) + assert "already claimed by" not in caplog.text + + def test_register_raises_for_empty_chart_type(): class _BadPlugin(BaseChartPlugin): chart_type = ""