diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index 8426a3f9811c..5fe4f5367d91 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -859,7 +859,7 @@ const SqlEditor: FC = ({ `} > {' '} - {t(`You are edting a query from the virtual dataset `) + + {t(`You are editing a query from the virtual dataset `) + queryEditor.name}

{ const permalink = location.pathname.match(/\/p\/\w+/)?.[0].slice(3); if (queryParams.size > 0 || permalink) { const autorun = queryParams.get('autorun') === 'true'; + const isDataset = queryParams.get('isDataset') === 'true'; const queryParamsState = { requestedQuery: { ...Object.fromEntries(queryParams), autorun, permalink, }, - isDataset: true, + isDataset, } as LocationState; return {children}; } diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index fc7af43aef55..8cc500ecac18 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -1029,6 +1029,13 @@ class ChartDataExtrasSchema(Schema): }, allow_none=True, ) + transpile_to_dialect = fields.Boolean( + metadata={ + "description": "If true, WHERE/HAVING clauses will be transpiled to the " + "target database dialect using SQLGlot." + }, + allow_none=True, + ) class AnnotationLayerSchema(Schema): diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py index 633e3c6a9c69..d3801e66885c 100644 --- a/superset/mcp_service/chart/chart_utils.py +++ b/superset/mcp_service/chart/chart_utils.py @@ -139,8 +139,9 @@ def map_table_config(config: TableChartConfig) -> Dict[str, Any]: if not raw_columns and not aggregated_metrics: raise ValueError("Table chart configuration resulted in no displayable columns") + # Use the viz_type from config (defaults to "table", can be "ag-grid-table") form_data: Dict[str, Any] = { - "viz_type": "table", + "viz_type": config.viz_type, } # Handle raw columns (no aggregation) @@ -370,7 +371,8 @@ def analyze_chart_capabilities(chart: Any | None, config: Any) -> ChartCapabilit } viz_type = viz_type_map.get(kind, "echarts_timeseries_line") elif chart_type == "table": - viz_type = "table" + # Use the viz_type from config if available (table or ag-grid-table) + viz_type = getattr(config, "viz_type", "table") else: viz_type = "unknown" @@ -382,10 +384,11 @@ def analyze_chart_capabilities(chart: Any | None, config: Any) -> ChartCapabilit "echarts_timeseries_scatter", "deck_scatter", "deck_hex", + "ag-grid-table", # AG Grid tables are interactive ] supports_interaction = viz_type in interactive_types - supports_drill_down = viz_type in ["table", "pivot_table_v2"] + supports_drill_down = viz_type in ["table", "pivot_table_v2", "ag-grid-table"] supports_real_time = viz_type in [ "echarts_timeseries_line", "echarts_timeseries_bar", @@ -433,7 +436,8 @@ def analyze_chart_semantics(chart: Any | None, config: Any) -> ChartSemantics: } viz_type = viz_type_map.get(kind, "echarts_timeseries_line") elif chart_type == "table": - viz_type = "table" + # Use the viz_type from config if available (table or ag-grid-table) + viz_type = getattr(config, "viz_type", "table") else: viz_type = "unknown" @@ -442,6 +446,10 @@ def analyze_chart_semantics(chart: Any | None, config: Any) -> ChartSemantics: "echarts_timeseries_line": "Shows trends and changes over time", "echarts_timeseries_bar": "Compares values across categories or time periods", "table": "Displays detailed data in tabular format", + "ag-grid-table": ( + "Interactive table with advanced features like column resizing, " + "sorting, filtering, and server-side pagination" + ), "pie": "Shows proportional relationships within a dataset", "echarts_area": "Emphasizes cumulative totals and part-to-whole relationships", } diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py index cecd34e85e71..f28d7e75f695 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), @@ -608,6 +608,14 @@ class TableChartConfig(BaseModel): chart_type: Literal["table"] = Field( ..., description="Chart type (REQUIRED: must be 'table')" ) + viz_type: Literal["table", "ag-grid-table"] = Field( + "table", + description=( + "Visualization type: 'table' for standard table, 'ag-grid-table' for " + "AG Grid Interactive Table with advanced features like column resizing, " + "sorting, filtering, and server-side pagination" + ), + ) columns: List[ColumnRef] = Field( ..., min_length=1, diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py index ba444ebb6eb1..9cd6e6225848 100644 --- a/superset/mcp_service/chart/tool/generate_chart.py +++ b/superset/mcp_service/chart/tool/generate_chart.py @@ -37,13 +37,9 @@ 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 c2c33a83eb93..2b3ecf353e7a 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 @@ def _analyze_field_types( 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 13b9630a0f2a..ed5355906ffa 100644 --- a/superset/mcp_service/chart/tool/update_chart.py +++ b/superset/mcp_service/chart/tool/update_chart.py @@ -38,10 +38,7 @@ 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 295d08ab20e1..271709e230fc 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 @@ 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 44eee2b6feda..55108c7387ce 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" diff --git a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py index 7292220cd75b..5bbae63912b1 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py @@ -54,6 +54,59 @@ def test_unique_labels_accepted(self) -> None: ) assert len(config.columns) == 2 + def test_default_viz_type_is_table(self) -> None: + """Test that default viz_type is 'table'.""" + config = TableChartConfig( + chart_type="table", + columns=[ColumnRef(name="product")], + ) + assert config.viz_type == "table" + + def test_ag_grid_table_viz_type_accepted(self) -> None: + """Test that viz_type='ag-grid-table' is accepted for AG Grid table.""" + config = TableChartConfig( + chart_type="table", + viz_type="ag-grid-table", + columns=[ + ColumnRef(name="product_line"), + ColumnRef(name="sales", aggregate="SUM", label="Total Sales"), + ], + ) + assert config.viz_type == "ag-grid-table" + assert len(config.columns) == 2 + + def test_ag_grid_table_with_all_options(self) -> None: + """Test AG Grid table with filters and sorting.""" + from superset.mcp_service.chart.schemas import FilterConfig + + config = TableChartConfig( + chart_type="table", + viz_type="ag-grid-table", + columns=[ + ColumnRef(name="product_line"), + ColumnRef(name="quantity", aggregate="SUM", label="Total Quantity"), + ColumnRef(name="sales", aggregate="SUM", label="Total Sales"), + ], + filters=[FilterConfig(column="status", op="=", value="active")], + sort_by=["product_line"], + ) + assert config.viz_type == "ag-grid-table" + assert len(config.columns) == 3 + assert config.filters is not None + assert len(config.filters) == 1 + assert config.sort_by == ["product_line"] + + def test_invalid_viz_type_rejected(self) -> None: + """Test that invalid viz_type values are rejected.""" + from pydantic import ValidationError + + with pytest.raises(ValidationError): + TableChartConfig( + chart_type="table", + viz_type="invalid-type", + columns=[ColumnRef(name="product")], + ) + class TestXYChartConfig: """Test XYChartConfig validation.""" diff --git a/tests/unit_tests/mcp_service/chart/test_chart_utils.py b/tests/unit_tests/mcp_service/chart/test_chart_utils.py index 560af4e8632a..4f4f3a3857bd 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_utils.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_utils.py @@ -152,6 +152,58 @@ def test_map_table_config_with_sort(self) -> None: result = map_table_config(config) assert result["order_by_cols"] == ["product", "revenue"] + def test_map_table_config_ag_grid_table(self) -> None: + """Test table config mapping with AG Grid Interactive Table viz_type""" + config = TableChartConfig( + chart_type="table", + viz_type="ag-grid-table", + columns=[ + ColumnRef(name="product_line"), + ColumnRef(name="sales", aggregate="SUM", label="Total Sales"), + ], + ) + + result = map_table_config(config) + + # AG Grid tables use 'ag-grid-table' viz_type + assert result["viz_type"] == "ag-grid-table" + assert result["query_mode"] == "aggregate" + assert len(result["metrics"]) == 1 + assert result["metrics"][0]["aggregate"] == "SUM" + # Non-aggregated columns should be in groupby + assert "groupby" in result + assert "product_line" in result["groupby"] + + def test_map_table_config_ag_grid_raw_mode(self) -> None: + """Test AG Grid table with raw columns (no aggregates)""" + config = TableChartConfig( + chart_type="table", + viz_type="ag-grid-table", + columns=[ + ColumnRef(name="product_line"), + ColumnRef(name="category"), + ColumnRef(name="region"), + ], + ) + + result = map_table_config(config) + + assert result["viz_type"] == "ag-grid-table" + assert result["query_mode"] == "raw" + assert result["all_columns"] == ["product_line", "category", "region"] + assert "metrics" not in result + + def test_map_table_config_default_viz_type(self) -> None: + """Test that default viz_type is 'table' not 'ag-grid-table'""" + config = TableChartConfig( + chart_type="table", + columns=[ColumnRef(name="product")], + ) + + result = map_table_config(config) + + assert result["viz_type"] == "table" + class TestMapXYConfig: """Test map_xy_config function"""