diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py index f8e9d8186074..ed1172f612cd 100644 --- a/superset/mcp_service/chart/schemas.py +++ b/superset/mcp_service/chart/schemas.py @@ -835,7 +835,7 @@ class GenerateChartRequest(QueryCacheControl): dataset_id: int | str = Field(..., description="Dataset identifier (ID, UUID)") config: ChartConfig = Field(..., description="Chart configuration") save_chart: bool = Field( - default=True, + default=False, description="Whether to permanently save the chart in Superset", ) generate_preview: bool = Field( @@ -862,6 +862,16 @@ def validate_cache_timeout(self) -> "GenerateChartRequest": ) return self + @model_validator(mode="after") + def validate_save_or_preview(self) -> "GenerateChartRequest": + """Ensure at least one of save_chart or generate_preview is enabled.""" + if not self.save_chart and not self.generate_preview: + raise ValueError( + "At least one of 'save_chart' or 'generate_preview' must be True. " + "A request with both set to False would be a no-op." + ) + return self + class GenerateExploreLinkRequest(FormDataCacheControl): dataset_id: int | str = Field(..., description="Dataset identifier (ID, UUID)") diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py index dfbc466a57fc..ba444ebb6eb1 100644 --- a/superset/mcp_service/chart/tool/generate_chart.py +++ b/superset/mcp_service/chart/tool/generate_chart.py @@ -54,11 +54,11 @@ async def generate_chart( # noqa: C901 request: GenerateChartRequest, ctx: Context ) -> GenerateChartResponse: - """Create and save a chart in Superset. + """Create a chart preview in Superset, optionally saving it permanently. IMPORTANT BEHAVIOR: - - Charts ARE saved by default (save_chart=True) - - Set save_chart=False for temporary preview only + - Charts are NOT saved by default (save_chart=False) - preview only + - Set save_chart=True to permanently save the chart - LLM clients MUST display returned chart URL to users - Embed preview_url as image: ![Chart Preview](preview_url) - Use numeric dataset ID or UUID (NOT schema.table_name format) diff --git a/tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py b/tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py index d5769c82d5d9..62bd9bbd22f1 100644 --- a/tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py +++ b/tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py @@ -182,24 +182,35 @@ async def test_dataset_id_flexibility(self): @pytest.mark.asyncio async def test_save_chart_flag(self): """Test save_chart flag behavior.""" - # Default should be True (save chart) + # Default should be False (preview only, not saved) request1 = GenerateChartRequest( dataset_id="1", config=TableChartConfig( chart_type="table", columns=[ColumnRef(name="col1")] ), ) - assert request1.save_chart is True + assert request1.save_chart is False - # Explicit False (preview only) + # Explicit True (save chart permanently) request2 = GenerateChartRequest( dataset_id="1", config=TableChartConfig( chart_type="table", columns=[ColumnRef(name="col1")] ), - save_chart=False, + save_chart=True, ) - assert request2.save_chart is False + assert request2.save_chart is True + + # Both False should raise validation error (no-op request) + with pytest.raises(ValueError, match="At least one of"): + GenerateChartRequest( + dataset_id="1", + config=TableChartConfig( + chart_type="table", columns=[ColumnRef(name="col1")] + ), + save_chart=False, + generate_preview=False, + ) @pytest.mark.asyncio async def test_preview_formats(self):