diff --git a/superset/mcp_service/dashboard/schemas.py b/superset/mcp_service/dashboard/schemas.py index 62e4b46a0ec..7b002677c69 100644 --- a/superset/mcp_service/dashboard/schemas.py +++ b/superset/mcp_service/dashboard/schemas.py @@ -451,7 +451,13 @@ class GenerateDashboardRequest(BaseModel): chart_ids: List[int] = Field( ..., description="List of chart IDs to include in the dashboard", min_length=1 ) - dashboard_title: str = Field(..., description="Title for the new dashboard") + dashboard_title: str | None = Field( + None, + description=( + "Title for the new dashboard. When omitted a descriptive title " + "is generated from the included chart names." + ), + ) description: str | None = Field(None, description="Description for the dashboard") published: bool = Field( default=True, description="Whether to publish the dashboard" diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py b/superset/mcp_service/dashboard/tool/generate_dashboard.py index a34c21beafb..8126ee387ca 100644 --- a/superset/mcp_service/dashboard/tool/generate_dashboard.py +++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py @@ -138,6 +138,45 @@ def _create_dashboard_layout(chart_objects: List[Any]) -> Dict[str, Any]: return layout +_DEFAULT_DASHBOARD_TITLE = "Dashboard" +_MAX_TITLE_LENGTH = 150 + + +def _generate_title_from_charts(chart_objects: List[Any]) -> str: + """ + Build a descriptive dashboard title from the included chart names. + + Joins up to three chart ``slice_name`` values with " & " (two charts) + or ", " (three charts). When there are more than three charts the + remaining count is appended as "+ N more". The result is capped at + ``_MAX_TITLE_LENGTH`` characters. + + Returns ``"Dashboard"`` when *chart_objects* is empty or no chart has + a usable name. + """ + names = [ + c.slice_name + for c in sorted(chart_objects, key=lambda c: getattr(c, "id", 0)) + if getattr(c, "slice_name", None) + ] + if not names: + return _DEFAULT_DASHBOARD_TITLE + + if len(names) == 1: + title = names[0] + elif len(names) == 2: + title = f"{names[0]} & {names[1]}" + elif len(names) == 3: + title = f"{names[0]}, {names[1]}, {names[2]}" + else: + title = f"{names[0]}, {names[1]}, {names[2]} + {len(names) - 3} more" + + if len(title) > _MAX_TITLE_LENGTH: + title = title[: _MAX_TITLE_LENGTH - 1] + "\u2026" + + return title + + @tool(tags=["mutate"]) @parse_request(GenerateDashboardRequest) def generate_dashboard( @@ -160,7 +199,10 @@ def generate_dashboard( with event_logger.log_context(action="mcp.generate_dashboard.chart_validation"): chart_objects = ( - db.session.query(Slice).filter(Slice.id.in_(request.chart_ids)).all() + db.session.query(Slice) + .filter(Slice.id.in_(request.chart_ids)) + .order_by(Slice.id) + .all() ) found_chart_ids = [chart.id for chart in chart_objects] @@ -177,10 +219,17 @@ def generate_dashboard( with event_logger.log_context(action="mcp.generate_dashboard.layout"): layout = _create_dashboard_layout(chart_objects) + # Resolve dashboard title: use provided title or derive from chart names + dashboard_title = ( + request.dashboard_title + if request.dashboard_title is not None + else _generate_title_from_charts(chart_objects) + ) + # Prepare dashboard data and create dashboard with event_logger.log_context(action="mcp.generate_dashboard.db_write"): dashboard_data = { - "dashboard_title": request.dashboard_title, + "dashboard_title": dashboard_title, "slug": None, # Let Superset auto-generate slug "css": "", "json_metadata": json.dumps( diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py index 83fe00df0fb..a0d3091744e 100644 --- a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py +++ b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py @@ -33,6 +33,9 @@ from superset.mcp_service.dashboard.tool.add_chart_to_existing_dashboard import _find_next_row_position, _find_tab_insert_target, ) +from superset.mcp_service.dashboard.tool.generate_dashboard import ( + _generate_title_from_charts, +) from superset.utils import json logging.basicConfig(level=logging.DEBUG) @@ -98,6 +101,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [ _mock_chart(id=1, slice_name="Sales Chart"), _mock_chart(id=2, slice_name="Revenue Chart"), @@ -136,6 +140,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [_mock_chart(id=1)] mock_db_session.query.return_value = mock_query @@ -159,6 +164,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [_mock_chart(id=5, slice_name="Single Chart")] mock_db_session.query.return_value = mock_query @@ -189,6 +195,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [ _mock_chart(id=i, slice_name=f"Chart {i}") for i in chart_ids ] @@ -258,6 +265,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [_mock_chart(id=1)] mock_db_session.query.return_value = mock_query mock_create_command.return_value.run.side_effect = Exception("Creation failed") @@ -281,6 +289,7 @@ class TestGenerateDashboard: mock_query = Mock() mock_filter = Mock() mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter mock_filter.all.return_value = [_mock_chart(id=3)] mock_db_session.query.return_value = mock_query @@ -307,6 +316,67 @@ class TestGenerateDashboard: "description" not in call_args or call_args.get("description") is None ) + @patch("superset.commands.dashboard.create.CreateDashboardCommand") + @patch("superset.db.session") + @pytest.mark.asyncio + async def test_generate_dashboard_auto_title_from_charts( + self, mock_db_session, mock_create_command, mcp_server + ): + """Test that omitting dashboard_title generates a title from chart names.""" + mock_query = Mock() + mock_filter = Mock() + mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter + mock_filter.all.return_value = [ + _mock_chart(id=1, slice_name="Sales Revenue"), + _mock_chart(id=2, slice_name="Customer Count"), + ] + mock_db_session.query.return_value = mock_query + + mock_dashboard = _mock_dashboard(id=50, title="Sales Revenue & Customer Count") + mock_create_command.return_value.run.return_value = mock_dashboard + + # No dashboard_title provided + request = {"chart_ids": [1, 2]} + + async with Client(mcp_server) as client: + result = await client.call_tool("generate_dashboard", {"request": request}) + + assert result.structured_content["error"] is None + + call_args = mock_create_command.call_args[0][0] + assert call_args["dashboard_title"] == "Sales Revenue & Customer Count" + + @patch("superset.commands.dashboard.create.CreateDashboardCommand") + @patch("superset.db.session") + @pytest.mark.asyncio + async def test_generate_dashboard_empty_string_title_preserved( + self, mock_db_session, mock_create_command, mcp_server + ): + """Test that an explicit empty-string title is NOT replaced by auto-gen.""" + mock_query = Mock() + mock_filter = Mock() + mock_query.filter.return_value = mock_filter + mock_filter.order_by.return_value = mock_filter + mock_filter.all.return_value = [ + _mock_chart(id=1, slice_name="Sales Revenue"), + ] + mock_db_session.query.return_value = mock_query + + mock_dashboard = _mock_dashboard(id=60, title="") + mock_create_command.return_value.run.return_value = mock_dashboard + + # Explicit empty string title + request = {"chart_ids": [1], "dashboard_title": ""} + + async with Client(mcp_server) as client: + result = await client.call_tool("generate_dashboard", {"request": request}) + + assert result.structured_content["error"] is None + + call_args = mock_create_command.call_args[0][0] + assert call_args["dashboard_title"] == "" + class TestAddChartToExistingDashboard: """Tests for add_chart_to_existing_dashboard MCP tool.""" @@ -766,3 +836,55 @@ class TestLayoutHelpers: assert "ROW-new" in layout["TAB-first"]["children"] assert "ROW-new" not in layout["GRID_ID"]["children"] + + +class TestGenerateTitleFromCharts: + """Tests for _generate_title_from_charts helper.""" + + def test_empty_list_returns_dashboard(self): + assert _generate_title_from_charts([]) == "Dashboard" + + def test_single_chart(self): + charts = [_mock_chart(id=1, slice_name="Revenue")] + assert _generate_title_from_charts(charts) == "Revenue" + + def test_two_charts_joined_with_ampersand(self): + charts = [ + _mock_chart(id=1, slice_name="Revenue"), + _mock_chart(id=2, slice_name="Costs"), + ] + assert _generate_title_from_charts(charts) == "Revenue & Costs" + + def test_three_charts_joined_with_commas(self): + charts = [ + _mock_chart(id=1, slice_name="Revenue"), + _mock_chart(id=2, slice_name="Costs"), + _mock_chart(id=3, slice_name="Profit"), + ] + assert _generate_title_from_charts(charts) == "Revenue, Costs, Profit" + + def test_four_charts_shows_plus_more(self): + charts = [_mock_chart(id=i, slice_name=f"Chart {i}") for i in range(1, 5)] + assert ( + _generate_title_from_charts(charts) == "Chart 1, Chart 2, Chart 3 + 1 more" + ) + + def test_many_charts_shows_plus_more(self): + charts = [_mock_chart(id=i, slice_name=f"Chart {i}") for i in range(1, 8)] + assert ( + _generate_title_from_charts(charts) == "Chart 1, Chart 2, Chart 3 + 4 more" + ) + + def test_charts_without_names_returns_dashboard(self): + chart = Mock() + chart.slice_name = None + assert _generate_title_from_charts([chart]) == "Dashboard" + + def test_long_title_is_truncated(self): + charts = [ + _mock_chart(id=1, slice_name="A" * 100), + _mock_chart(id=2, slice_name="B" * 100), + ] + title = _generate_title_from_charts(charts) + assert len(title) <= 150 + assert title.endswith("\u2026")