diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py index cecd34e85e7..1f6a5a0d34a 100644 --- a/superset/mcp_service/chart/schemas.py +++ b/superset/mcp_service/chart/schemas.py @@ -211,13 +211,13 @@ def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None: if not chart: return None - # Generate MCP service screenshot URL instead of chart's native URL - from superset.mcp_service.utils.url_utils import get_chart_screenshot_url + # Use the chart's native URL (explore URL) instead of screenshot URL + from superset.mcp_service.utils.url_utils import get_superset_base_url chart_id = getattr(chart, "id", None) - screenshot_url = None + chart_url = None if chart_id: - screenshot_url = get_chart_screenshot_url(chart_id) + chart_url = f"{get_superset_base_url()}/explore/?slice_id={chart_id}" return ChartInfo( id=chart_id, @@ -225,7 +225,7 @@ def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None: viz_type=getattr(chart, "viz_type", None), datasource_name=getattr(chart, "datasource_name", None), datasource_type=getattr(chart, "datasource_type", None), - url=screenshot_url, + url=chart_url, description=getattr(chart, "description", None), cache_timeout=getattr(chart, "cache_timeout", None), form_data=getattr(chart, "form_data", None), diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py index ba444ebb6eb..9cd6e622584 100644 --- a/superset/mcp_service/chart/tool/generate_chart.py +++ b/superset/mcp_service/chart/tool/generate_chart.py @@ -37,13 +37,9 @@ from superset.mcp_service.chart.schemas import ( GenerateChartRequest, GenerateChartResponse, PerformanceMetadata, - URLPreview, ) from superset.mcp_service.utils.schema_utils import parse_request -from superset.mcp_service.utils.url_utils import ( - get_chart_screenshot_url, - get_superset_base_url, -) +from superset.mcp_service.utils.url_utils import get_superset_base_url from superset.utils import json logger = logging.getLogger(__name__) @@ -60,7 +56,6 @@ async def generate_chart( # noqa: C901 - 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) - MUST include chart_type in config (either 'xy' or 'table') @@ -397,20 +392,9 @@ async def generate_chart( # noqa: C901 previews[format_type] = preview_result.content else: # For preview-only mode (save_chart=false) - if format_type == "url" and form_data_key: - # Generate screenshot URL using centralized helper - from superset.mcp_service.utils.url_utils import ( - get_explore_screenshot_url, - ) - - preview_url = get_explore_screenshot_url(form_data_key) - previews[format_type] = URLPreview( - preview_url=preview_url, - width=800, - height=600, - supports_interaction=False, - ) - elif format_type in ["ascii", "table", "vega_lite"]: + # Note: Screenshot-based URL previews are not supported. + # Use the explore_url to view the chart interactively. + if format_type in ["ascii", "table", "vega_lite"]: # Generate preview from form data without saved chart from superset.mcp_service.chart.preview_utils import ( generate_preview_from_form_data, @@ -483,7 +467,6 @@ async def generate_chart( # noqa: C901 "data": f"{get_superset_base_url()}/api/v1/chart/{chart.id}/data/" if chart else None, - "preview": get_chart_screenshot_url(chart.id) if chart else None, "export": f"{get_superset_base_url()}/api/v1/chart/{chart.id}/export/" if chart else None, diff --git a/superset/mcp_service/chart/tool/get_chart_preview.py b/superset/mcp_service/chart/tool/get_chart_preview.py index c2c33a83eb9..2b3ecf353e7 100644 --- a/superset/mcp_service/chart/tool/get_chart_preview.py +++ b/superset/mcp_service/chart/tool/get_chart_preview.py @@ -72,53 +72,17 @@ class URLPreviewStrategy(PreviewFormatStrategy): """Generate URL-based image preview.""" def generate(self) -> URLPreview | ChartError: - try: - from flask import g - - from superset.mcp_service.screenshot.pooled_screenshot import ( - PooledChartScreenshot, - ) - from superset.mcp_service.utils.url_utils import get_superset_base_url - - # Check if chart.id is None - if self.chart.id is None: - return ChartError( - error="Chart has no ID - cannot generate URL preview", - error_type="InvalidChart", - ) - - # Use configured Superset base URL instead of Flask's url_for - # which may not respect SUPERSET_WEBSERVER_ADDRESS - base_url = get_superset_base_url() - chart_url = f"{base_url}/superset/slice/{self.chart.id}/" - screenshot = PooledChartScreenshot(chart_url, self.chart.digest) - - window_size = (self.request.width or 800, self.request.height or 600) - image_data = screenshot.get_screenshot(user=g.user, window_size=window_size) - - if image_data: - # Use the MCP service screenshot URL via centralized helper - from superset.mcp_service.utils.url_utils import ( - get_chart_screenshot_url, - ) - - preview_url = get_chart_screenshot_url(self.chart.id) - - return URLPreview( - preview_url=preview_url, - width=self.request.width or 800, - height=self.request.height or 600, - ) - else: - return ChartError( - error=f"Could not generate screenshot for chart {self.chart.id}", - error_type="ScreenshotError", - ) - except Exception as e: - logger.error("URL preview generation failed: %s", e) - return ChartError( - error=f"Failed to generate URL preview: {str(e)}", error_type="URLError" - ) + # Screenshot-based URL previews are not supported. + # Users should use the explore_url to view the chart interactively, + # or use other preview formats like 'ascii', 'table', or 'vega_lite'. + return ChartError( + error=( + "URL-based screenshot previews are not supported. " + "Use the explore_url to view the chart interactively, " + "or try formats: 'ascii', 'table', or 'vega_lite'." + ), + error_type="UnsupportedFormat", + ) # Base64 preview support removed - we never return base64 data @@ -479,7 +443,7 @@ class VegaLitePreviewStrategy(PreviewFormatStrategy): except Exception as e: logger.warning("Error in field type analysis: %s", e) # Return nominal types for all fields as fallback - return {field: "nominal" for field in fields} + return dict.fromkeys(fields, "nominal") return field_types diff --git a/superset/mcp_service/chart/tool/update_chart.py b/superset/mcp_service/chart/tool/update_chart.py index 13b9630a0f2..ed5355906ff 100644 --- a/superset/mcp_service/chart/tool/update_chart.py +++ b/superset/mcp_service/chart/tool/update_chart.py @@ -38,10 +38,7 @@ from superset.mcp_service.chart.schemas import ( UpdateChartRequest, ) from superset.mcp_service.utils.schema_utils import parse_request -from superset.mcp_service.utils.url_utils import ( - get_chart_screenshot_url, - get_superset_base_url, -) +from superset.mcp_service.utils.url_utils import get_superset_base_url from superset.utils import json logger = logging.getLogger(__name__) @@ -57,7 +54,6 @@ async def update_chart( IMPORTANT: - Chart must already be saved (from generate_chart with save_chart=True) - LLM clients MUST display updated chart URL to users - - Embed preview_url as image: ![Updated Chart](preview_url) - Use numeric ID or UUID string to identify the chart (NOT chart name) - MUST include chart_type in config (either 'xy' or 'table') @@ -222,7 +218,6 @@ async def update_chart( "data": ( f"{get_superset_base_url()}/api/v1/chart/{updated_chart.id}/data/" ), - "preview": get_chart_screenshot_url(updated_chart.id), "export": ( f"{get_superset_base_url()}/api/v1/chart/{updated_chart.id}/export/" ), diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py b/superset/mcp_service/chart/tool/update_chart_preview.py index 295d08ab20e..271709e230f 100644 --- a/superset/mcp_service/chart/tool/update_chart_preview.py +++ b/superset/mcp_service/chart/tool/update_chart_preview.py @@ -37,10 +37,8 @@ from superset.mcp_service.chart.schemas import ( AccessibilityMetadata, PerformanceMetadata, UpdateChartPreviewRequest, - URLPreview, ) from superset.mcp_service.utils.schema_utils import parse_request -from superset.mcp_service.utils.url_utils import get_mcp_service_url logger = logging.getLogger(__name__) @@ -56,7 +54,6 @@ def update_chart_preview( - Modifies cached form_data from generate_chart (save_chart=False) - Original form_data_key is invalidated, new one returned - LLM clients MUST display explore_url to users - - Embed preview_url as image: ![Chart Preview](preview_url) Use when: - Modifying preview before deciding to save @@ -99,30 +96,9 @@ def update_chart_preview( high_contrast_available=False, ) - # Generate previews if requested - previews = {} - if request.generate_preview and new_form_data_key: - try: - for format_type in request.preview_formats: - if format_type == "url": - # Generate screenshot URL using new form_data key - mcp_base = get_mcp_service_url() - preview_url = ( - f"{mcp_base}/screenshot/explore/{new_form_data_key}.png" - ) - - previews[format_type] = URLPreview( - preview_url=preview_url, - width=800, - height=600, - supports_interaction=False, - ) - # Other formats would need form_data execution - # which is more complex for preview-only mode - - except Exception as e: - # Log warning but don't fail the entire request - logger.warning("Preview generation failed: %s", e) + # Note: Screenshot-based previews are not supported. + # Use the explore_url to view the chart interactively. + previews: Dict[str, Any] = {} # Return enhanced data result = { diff --git a/superset/mcp_service/utils/url_utils.py b/superset/mcp_service/utils/url_utils.py index 44eee2b6fed..55108c7387c 100644 --- a/superset/mcp_service/utils/url_utils.py +++ b/superset/mcp_service/utils/url_utils.py @@ -89,31 +89,3 @@ def get_mcp_service_url() -> str: # Development fallback - direct access to MCP service on port 5008 return "http://localhost:5008" - - -def get_chart_screenshot_url(chart_id: int | str) -> str: - """ - Generate a screenshot URL for a chart using the MCP service. - - Args: - chart_id: Chart ID (numeric or string) - - Returns: - Complete URL to the chart screenshot endpoint - """ - mcp_base = get_mcp_service_url() - return f"{mcp_base}/screenshot/chart/{chart_id}.png" - - -def get_explore_screenshot_url(form_data_key: str) -> str: - """ - Generate a screenshot URL for an explore view using the MCP service. - - Args: - form_data_key: Form data key for the explore view - - Returns: - Complete URL to the explore screenshot endpoint - """ - mcp_base = get_mcp_service_url() - return f"{mcp_base}/screenshot/explore/{form_data_key}.png"