Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion superset/mcp_service/chart/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)")
Expand Down
6 changes: 3 additions & 3 deletions superset/mcp_service/chart/tool/generate_chart.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 16 additions & 5 deletions tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading