From 139eea92f652cb599441e1a9a5dfd8732b3597d5 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Thu, 7 May 2026 03:10:30 +0000 Subject: [PATCH] refactor(mcp): eliminate dead code and complete plugin registry dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H1: Delete 7 dead _pre_validate_* static methods from SchemaValidator — exact duplicates of plugin pre_validate() methods, never called after _pre_validate_chart_type() was updated to delegate to plugin. H2: Inline DatasetValidator._normalize_xy_config/_normalize_table_config into XYChartPlugin/TableChartPlugin.normalize_column_refs() and delete both DatasetValidator helper methods. The 5 other plugins already called _get_canonical_column_name directly; XY and Table now match. H3: Add generate_name()/resolve_viz_type() to ChartTypePlugin protocol and BaseChartPlugin, implement in all 7 plugins. Replace the 7-arm isinstance chain in generate_chart_name() and the 7-arm elif chain in _resolve_viz_type() with single-line registry dispatch. H4: Add a sync comment above _CHART_TYPE_ERROR_HINTS to document that it must stay in sync with the plugin registry. M4: Move logger=logging.getLogger(__name__) from inside XYChartPlugin.get_runtime_warnings() to module scope. --- superset/mcp_service/chart/chart_utils.py | 73 +--- superset/mcp_service/chart/plugin.py | 34 ++ .../mcp_service/chart/plugins/big_number.py | 17 + .../mcp_service/chart/plugins/handlebars.py | 13 + .../chart/plugins/mixed_timeseries.py | 13 + superset/mcp_service/chart/plugins/pie.py | 13 + .../mcp_service/chart/plugins/pivot_table.py | 13 + superset/mcp_service/chart/plugins/table.py | 19 +- superset/mcp_service/chart/plugins/xy.py | 37 +- .../chart/validation/dataset_validator.py | 36 -- .../chart/validation/schema_validator.py | 349 +----------------- 11 files changed, 170 insertions(+), 447 deletions(-) diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py index 655c95a5f4d..31fc9537051 100644 --- a/superset/mcp_service/chart/chart_utils.py +++ b/superset/mcp_service/chart/chart_utils.py @@ -1132,13 +1132,7 @@ def _big_number_chart_what(config: BigNumberChartConfig) -> str: def generate_chart_name( - config: TableChartConfig - | XYChartConfig - | PieChartConfig - | PivotTableChartConfig - | MixedTimeseriesChartConfig - | HandlebarsChartConfig - | BigNumberChartConfig, + config: Any, dataset_name: str | None = None, ) -> str: """Generate a descriptive chart name following a standard format. @@ -1154,65 +1148,22 @@ def generate_chart_name( An en-dash followed by context (filters / time grain) is appended when such information is available. """ - if isinstance(config, TableChartConfig): - what = _table_chart_what(config, dataset_name) - context = _summarize_filters(config.filters) - elif isinstance(config, XYChartConfig): - what = _xy_chart_what(config) - context = _xy_chart_context(config) - elif isinstance(config, PieChartConfig): - what = _pie_chart_what(config) - context = _summarize_filters(config.filters) - elif isinstance(config, PivotTableChartConfig): - what = _pivot_table_what(config) - context = _summarize_filters(config.filters) - elif isinstance(config, MixedTimeseriesChartConfig): - what = _mixed_timeseries_what(config) - context = _summarize_filters(config.filters) - elif isinstance(config, HandlebarsChartConfig): - what = _handlebars_chart_what(config) - context = _summarize_filters(getattr(config, "filters", None)) - elif isinstance(config, BigNumberChartConfig): - what = _big_number_chart_what(config) - context = _summarize_filters(getattr(config, "filters", None)) - else: - return "Chart" + from superset.mcp_service.chart.registry import get_registry - name = what - if context: - name = f"{what} \u2013 {context}" - return _truncate(name) + plugin = get_registry().get(getattr(config, "chart_type", "")) + if plugin is None: + return "Chart" + return _truncate(plugin.generate_name(config, dataset_name)) def _resolve_viz_type(config: Any) -> str: """Resolve the Superset viz_type from a chart config object.""" - chart_type = getattr(config, "chart_type", "unknown") - if chart_type == "xy": - kind = getattr(config, "kind", "line") - viz_type_map = { - "line": "echarts_timeseries_line", - "bar": "echarts_timeseries_bar", - "area": "echarts_area", - "scatter": "echarts_timeseries_scatter", - } - return viz_type_map.get(kind, "echarts_timeseries_line") - elif chart_type == "table": - return getattr(config, "viz_type", "table") - elif chart_type == "pie": - return "pie" - elif chart_type == "pivot_table": - return "pivot_table_v2" - elif chart_type == "mixed_timeseries": - return "mixed_timeseries" - elif chart_type == "handlebars": - return "handlebars" - elif chart_type == "big_number": - show_trendline = getattr(config, "show_trendline", False) - temporal_column = getattr(config, "temporal_column", None) - return ( - "big_number" if show_trendline and temporal_column else "big_number_total" - ) - return "unknown" + from superset.mcp_service.chart.registry import get_registry + + plugin = get_registry().get(getattr(config, "chart_type", "")) + if plugin is None: + return "unknown" + return plugin.resolve_viz_type(config) def analyze_chart_capabilities(chart: Any | None, config: Any) -> ChartCapabilities: diff --git a/superset/mcp_service/chart/plugin.py b/superset/mcp_service/chart/plugin.py index 0221c079032..b310bc1da07 100644 --- a/superset/mcp_service/chart/plugin.py +++ b/superset/mcp_service/chart/plugin.py @@ -148,6 +148,30 @@ class ChartTypePlugin(Protocol): """ ... + def generate_name( + self, + config: Any, + dataset_name: str | None = None, + ) -> str: + """ + Return a descriptive chart name for the given config. + + Called by chart_utils.generate_chart_name(). The name should follow + the standard format conventions documented in that function. Plugins + that do not override this return the generic fallback "Chart". + """ + ... + + def resolve_viz_type(self, config: Any) -> str: + """ + Return the Superset-internal viz_type string for this config. + + Called by chart_utils._resolve_viz_type(). The returned string must + match a registered Superset viz plugin (e.g. "echarts_timeseries_line"). + Plugins that do not override this return "unknown". + """ + ... + class BaseChartPlugin: """ @@ -202,3 +226,13 @@ class BaseChartPlugin: dataset_id: int | str, ) -> list[str]: return [] + + def generate_name( + self, + config: Any, + dataset_name: str | None = None, + ) -> str: + return "Chart" + + def resolve_viz_type(self, config: Any) -> str: + return "unknown" diff --git a/superset/mcp_service/chart/plugins/big_number.py b/superset/mcp_service/chart/plugins/big_number.py index 6c7fc199f28..d924cd7735f 100644 --- a/superset/mcp_service/chart/plugins/big_number.py +++ b/superset/mcp_service/chart/plugins/big_number.py @@ -176,6 +176,23 @@ class BigNumberChartPlugin(BaseChartPlugin): return None + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _big_number_chart_what, + _summarize_filters, + ) + + what = _big_number_chart_what(config) + context = _summarize_filters(getattr(config, "filters", None)) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + show_trendline = getattr(config, "show_trendline", False) + temporal_column = getattr(config, "temporal_column", None) + if show_trendline and temporal_column: + return "big_number" + return "big_number_total" + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import BigNumberChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( diff --git a/superset/mcp_service/chart/plugins/handlebars.py b/superset/mcp_service/chart/plugins/handlebars.py index 918110a0c37..d177ede9457 100644 --- a/superset/mcp_service/chart/plugins/handlebars.py +++ b/superset/mcp_service/chart/plugins/handlebars.py @@ -142,6 +142,19 @@ class HandlebarsChartPlugin(BaseChartPlugin): return map_handlebars_config(config) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _handlebars_chart_what, + _summarize_filters, + ) + + what = _handlebars_chart_what(config) + context = _summarize_filters(getattr(config, "filters", None)) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + return "handlebars" + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import HandlebarsChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( diff --git a/superset/mcp_service/chart/plugins/mixed_timeseries.py b/superset/mcp_service/chart/plugins/mixed_timeseries.py index 1560948701e..706df74499d 100644 --- a/superset/mcp_service/chart/plugins/mixed_timeseries.py +++ b/superset/mcp_service/chart/plugins/mixed_timeseries.py @@ -110,6 +110,19 @@ class MixedTimeseriesChartPlugin(BaseChartPlugin): return map_mixed_timeseries_config(config, dataset_id=dataset_id) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _mixed_timeseries_what, + _summarize_filters, + ) + + what = _mixed_timeseries_what(config) + context = _summarize_filters(config.filters) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + return "mixed_timeseries" + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import MixedTimeseriesChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( diff --git a/superset/mcp_service/chart/plugins/pie.py b/superset/mcp_service/chart/plugins/pie.py index 132e169b66d..0160e134a3d 100644 --- a/superset/mcp_service/chart/plugins/pie.py +++ b/superset/mcp_service/chart/plugins/pie.py @@ -84,6 +84,19 @@ class PieChartPlugin(BaseChartPlugin): return map_pie_config(config) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _pie_chart_what, + _summarize_filters, + ) + + what = _pie_chart_what(config) + context = _summarize_filters(config.filters) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + return "pie" + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import PieChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( diff --git a/superset/mcp_service/chart/plugins/pivot_table.py b/superset/mcp_service/chart/plugins/pivot_table.py index c9b55c59115..e514a3307a0 100644 --- a/superset/mcp_service/chart/plugins/pivot_table.py +++ b/superset/mcp_service/chart/plugins/pivot_table.py @@ -107,6 +107,19 @@ class PivotTableChartPlugin(BaseChartPlugin): return map_pivot_table_config(config) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _pivot_table_what, + _summarize_filters, + ) + + what = _pivot_table_what(config) + context = _summarize_filters(config.filters) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + return "pivot_table_v2" + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import PivotTableChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( diff --git a/superset/mcp_service/chart/plugins/table.py b/superset/mcp_service/chart/plugins/table.py index 16152ef9baf..ee7d9f27d4b 100644 --- a/superset/mcp_service/chart/plugins/table.py +++ b/superset/mcp_service/chart/plugins/table.py @@ -89,6 +89,19 @@ class TableChartPlugin(BaseChartPlugin): return map_table_config(config) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _summarize_filters, + _table_chart_what, + ) + + what = _table_chart_what(config, dataset_name) + context = _summarize_filters(config.filters) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + return getattr(config, "viz_type", "table") + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: from superset.mcp_service.chart.schemas import TableChartConfig from superset.mcp_service.chart.validation.dataset_validator import ( @@ -96,6 +109,10 @@ class TableChartPlugin(BaseChartPlugin): ) config_dict = config.model_dump() - DatasetValidator._normalize_table_config(config_dict, dataset_context) + get_canonical = DatasetValidator._get_canonical_column_name + + for col in config_dict.get("columns") or []: + col["name"] = get_canonical(col["name"], dataset_context) + DatasetValidator._normalize_filters(config_dict, dataset_context) return TableChartConfig.model_validate(config_dict) diff --git a/superset/mcp_service/chart/plugins/xy.py b/superset/mcp_service/chart/plugins/xy.py index 75eea3f124d..92088ed66ab 100644 --- a/superset/mcp_service/chart/plugins/xy.py +++ b/superset/mcp_service/chart/plugins/xy.py @@ -19,12 +19,15 @@ from __future__ import annotations +import logging from typing import Any from superset.mcp_service.chart.plugin import BaseChartPlugin from superset.mcp_service.chart.schemas import ColumnRef from superset.mcp_service.common.error_schemas import ChartGenerationError +logger = logging.getLogger(__name__) + class XYChartPlugin(BaseChartPlugin): """Plugin for xy chart type (line, bar, area, scatter).""" @@ -105,17 +108,43 @@ class XYChartPlugin(BaseChartPlugin): ) config_dict = config.model_dump() - DatasetValidator._normalize_xy_config(config_dict, dataset_context) + get_canonical = DatasetValidator._get_canonical_column_name + + if config_dict.get("x"): + config_dict["x"]["name"] = get_canonical( + config_dict["x"]["name"], dataset_context + ) + for y_col in config_dict.get("y") or []: + y_col["name"] = get_canonical(y_col["name"], dataset_context) + for gb_col in config_dict.get("group_by") or []: + gb_col["name"] = get_canonical(gb_col["name"], dataset_context) + DatasetValidator._normalize_filters(config_dict, dataset_context) return XYChartConfig.model_validate(config_dict) + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: + from superset.mcp_service.chart.chart_utils import ( + _xy_chart_context, + _xy_chart_what, + ) + + what = _xy_chart_what(config) + context = _xy_chart_context(config) + return f"{what} \u2013 {context}" if context else what + + def resolve_viz_type(self, config: Any) -> str: + kind = getattr(config, "kind", "line") + return { + "line": "echarts_timeseries_line", + "bar": "echarts_timeseries_bar", + "area": "echarts_area", + "scatter": "echarts_timeseries_scatter", + }.get(kind, "echarts_timeseries_line") + def get_runtime_warnings(self, config: Any, dataset_id: int | str) -> list[str]: """Return format-compatibility and cardinality warnings for XY charts.""" - import logging - from superset.mcp_service.chart.schemas import XYChartConfig - logger = logging.getLogger(__name__) if not isinstance(config, XYChartConfig): return [] diff --git a/superset/mcp_service/chart/validation/dataset_validator.py b/superset/mcp_service/chart/validation/dataset_validator.py index b4dbbc489fc..f1ef0eacabf 100644 --- a/superset/mcp_service/chart/validation/dataset_validator.py +++ b/superset/mcp_service/chart/validation/dataset_validator.py @@ -330,42 +330,6 @@ class DatasetValidator: # Return original if not found (validation should catch this case) return column_name - @staticmethod - def _normalize_xy_config( - config_dict: Dict[str, Any], dataset_context: DatasetContext - ) -> None: - """Normalize column names in an XY chart config dict in place.""" - # Normalize x-axis column - if "x" in config_dict and config_dict["x"]: - config_dict["x"]["name"] = DatasetValidator._get_canonical_column_name( - config_dict["x"]["name"], dataset_context - ) - - # Normalize y-axis columns - if "y" in config_dict and config_dict["y"]: - for y_col in config_dict["y"]: - y_col["name"] = DatasetValidator._get_canonical_column_name( - y_col["name"], dataset_context - ) - - # Normalize group_by columns - if "group_by" in config_dict and config_dict["group_by"]: - for gb_col in config_dict["group_by"]: - gb_col["name"] = DatasetValidator._get_canonical_column_name( - gb_col["name"], dataset_context - ) - - @staticmethod - def _normalize_table_config( - config_dict: Dict[str, Any], dataset_context: DatasetContext - ) -> None: - """Normalize column names in a table chart config dict in place.""" - if "columns" in config_dict and config_dict["columns"]: - for col in config_dict["columns"]: - col["name"] = DatasetValidator._get_canonical_column_name( - col["name"], dataset_context - ) - @staticmethod def _normalize_filters( config_dict: Dict[str, Any], dataset_context: DatasetContext diff --git a/superset/mcp_service/chart/validation/schema_validator.py b/superset/mcp_service/chart/validation/schema_validator.py index d5ee9651a11..969779c26a8 100644 --- a/superset/mcp_service/chart/validation/schema_validator.py +++ b/superset/mcp_service/chart/validation/schema_validator.py @@ -186,353 +186,12 @@ class SchemaValidator: return False, error return True, None - @staticmethod - def _pre_validate_xy_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate XY chart configuration.""" - # x is optional — defaults to dataset's main_dttm_col in map_xy_config - if "y" not in config: - return False, ChartGenerationError( - error_type="missing_xy_fields", - message="XY chart missing required field: 'y' (Y-axis metrics)", - details="XY charts require Y-axis (metrics) specifications. " - "X-axis is optional and defaults to the dataset's primary " - "datetime column when omitted.", - suggestions=[ - "Add 'y' field: [{'name': 'metric_column', 'aggregate': 'SUM'}] " - "for Y-axis", - "Example: {'chart_type': 'xy', 'x': {'name': 'date'}, " - "'y': [{'name': 'sales', 'aggregate': 'SUM'}]}", - ], - error_code="MISSING_XY_FIELDS", - ) - - # Validate Y is a list - if not isinstance(config.get("y", []), list): - return False, ChartGenerationError( - error_type="invalid_y_format", - message="Y-axis must be a list of metrics", - details="The 'y' field must be an array of metric specifications", - suggestions=[ - "Wrap Y-axis metric in array: 'y': [{'name': 'column', " - "'aggregate': 'SUM'}]", - "Multiple metrics supported: 'y': [metric1, metric2, ...]", - ], - error_code="INVALID_Y_FORMAT", - ) - - return True, None - - @staticmethod - def _pre_validate_table_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate table chart configuration.""" - if "columns" not in config: - return False, ChartGenerationError( - error_type="missing_columns", - message="Table chart missing required field: columns", - details="Table charts require a 'columns' array to specify which " - "columns to display", - suggestions=[ - "Add 'columns' field with array of column specifications", - "Example: 'columns': [{'name': 'product'}, {'name': 'sales', " - "'aggregate': 'SUM'}]", - "Each column can have optional 'aggregate' for metrics", - ], - error_code="MISSING_COLUMNS", - ) - - if not isinstance(config.get("columns", []), list): - return False, ChartGenerationError( - error_type="invalid_columns_format", - message="Columns must be a list", - details="The 'columns' field must be an array of column specifications", - suggestions=[ - "Ensure columns is an array: 'columns': [...]", - "Each column should be an object with 'name' field", - ], - error_code="INVALID_COLUMNS_FORMAT", - ) - - return True, None - - @staticmethod - def _pre_validate_pie_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate pie chart configuration.""" - missing_fields = [] - - if "dimension" not in config: - missing_fields.append("'dimension' (category column for slices)") - if "metric" not in config: - missing_fields.append("'metric' (value metric for slice sizes)") - - if missing_fields: - return False, ChartGenerationError( - error_type="missing_pie_fields", - message=f"Pie chart missing required " - f"fields: {', '.join(missing_fields)}", - details="Pie charts require a dimension (categories) and a metric " - "(values)", - suggestions=[ - "Add 'dimension' field: {'name': 'category_column'}", - "Add 'metric' field: {'name': 'value_column', 'aggregate': 'SUM'}", - "Example: {'chart_type': 'pie', 'dimension': {'name': " - "'product'}, 'metric': {'name': 'revenue', 'aggregate': 'SUM'}}", - ], - error_code="MISSING_PIE_FIELDS", - ) - - return True, None - - @staticmethod - def _pre_validate_handlebars_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate handlebars chart configuration.""" - if "handlebars_template" not in config: - return False, ChartGenerationError( - error_type="missing_handlebars_template", - message="Handlebars chart missing required field: handlebars_template", - details="Handlebars charts require a 'handlebars_template' string " - "containing Handlebars HTML template markup", - suggestions=[ - "Add 'handlebars_template' with a Handlebars HTML template", - "Data is available as {{data}} array in the template", - "Example: ''", - ], - error_code="MISSING_HANDLEBARS_TEMPLATE", - ) - - template = config.get("handlebars_template") - if not isinstance(template, str) or not template.strip(): - return False, ChartGenerationError( - error_type="invalid_handlebars_template", - message="Handlebars template must be a non-empty string", - details="The 'handlebars_template' field must be a non-empty string " - "containing valid Handlebars HTML template markup", - suggestions=[ - "Ensure handlebars_template is a non-empty string", - "Example: ''", - ], - error_code="INVALID_HANDLEBARS_TEMPLATE", - ) - - query_mode = config.get("query_mode", "aggregate") - if query_mode not in ("aggregate", "raw"): - return False, ChartGenerationError( - error_type="invalid_query_mode", - message="Invalid query_mode for handlebars chart", - details="query_mode must be either 'aggregate' or 'raw'", - suggestions=[ - "Use 'aggregate' for aggregated data (default)", - "Use 'raw' for individual rows", - ], - error_code="INVALID_QUERY_MODE", - ) - - if query_mode == "raw" and not config.get("columns"): - return False, ChartGenerationError( - error_type="missing_raw_columns", - message="Handlebars chart in 'raw' mode requires 'columns'", - details="When query_mode is 'raw', you must specify which columns " - "to include in the query results", - suggestions=[ - "Add 'columns': [{'name': 'column_name'}] for raw mode", - "Or use query_mode='aggregate' with 'metrics' " - "and optional 'groupby'", - ], - error_code="MISSING_RAW_COLUMNS", - ) - - if query_mode == "aggregate" and not config.get("metrics"): - return False, ChartGenerationError( - error_type="missing_aggregate_metrics", - message="Handlebars chart in 'aggregate' mode requires 'metrics'", - details="When query_mode is 'aggregate' (default), you must specify " - "at least one metric with an aggregate function", - suggestions=[ - "Add 'metrics': [{'name': 'column', 'aggregate': 'SUM'}]", - "Or use query_mode='raw' with 'columns' for individual rows", - ], - error_code="MISSING_AGGREGATE_METRICS", - ) - - return True, None - - @staticmethod - def _pre_validate_big_number_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate big number chart configuration.""" - if "metric" not in config: - return False, ChartGenerationError( - error_type="missing_metric", - message="Big Number chart missing required field: metric", - details="Big Number charts require a 'metric' field " - "specifying the value to display", - suggestions=[ - "Add 'metric' with name and aggregate: " - "{'name': 'revenue', 'aggregate': 'SUM'}", - "The aggregate function is required (SUM, COUNT, AVG, MIN, MAX)", - "Example: {'chart_type': 'big_number', " - "'metric': {'name': 'sales', 'aggregate': 'SUM'}}", - ], - error_code="MISSING_BIG_NUMBER_METRIC", - ) - - metric = config.get("metric", {}) - if not isinstance(metric, dict): - return False, ChartGenerationError( - error_type="invalid_metric_type", - message="Big Number metric must be a dict with 'name' and 'aggregate'", - details="The 'metric' field must be an object, " - f"got {type(metric).__name__}", - suggestions=[ - "Use a dict: {'name': 'col', 'aggregate': 'SUM'}", - "Valid aggregates: SUM, COUNT, AVG, MIN, MAX", - ], - error_code="INVALID_BIG_NUMBER_METRIC_TYPE", - ) - if not metric.get("aggregate") and not metric.get("saved_metric"): - return False, ChartGenerationError( - error_type="missing_metric_aggregate", - message="Big Number metric must include an aggregate function " - "or reference a saved metric", - details="The metric must have an 'aggregate' field " - "or 'saved_metric': true", - suggestions=[ - "Add 'aggregate' to your metric: " - "{'name': 'col', 'aggregate': 'SUM'}", - "Or use a saved metric: " - "{'name': 'total_sales', 'saved_metric': true}", - "Valid aggregates: SUM, COUNT, AVG, MIN, MAX", - ], - error_code="MISSING_BIG_NUMBER_AGGREGATE", - ) - - show_trendline = config.get("show_trendline", False) - temporal_column = config.get("temporal_column") - if show_trendline and not temporal_column: - return False, ChartGenerationError( - error_type="missing_temporal_column", - message="Trendline requires a temporal column", - details="When 'show_trendline' is True, a " - "'temporal_column' must be specified", - suggestions=[ - "Add 'temporal_column': 'date_column_name'", - "Or set 'show_trendline': false for number only", - "Use get_dataset_info to find temporal columns", - ], - error_code="MISSING_TEMPORAL_COLUMN", - ) - - return True, None - - @staticmethod - def _pre_validate_pivot_table_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate pivot table configuration.""" - missing_fields = [] - - if "rows" not in config: - missing_fields.append("'rows' (row grouping columns)") - if "metrics" not in config: - missing_fields.append("'metrics' (aggregation metrics)") - - if missing_fields: - return False, ChartGenerationError( - error_type="missing_pivot_fields", - message=f"Pivot table missing required " - f"fields: {', '.join(missing_fields)}", - details="Pivot tables require row groupings and metrics", - suggestions=[ - "Add 'rows' field: [{'name': 'category'}]", - "Add 'metrics' field: [{'name': 'sales', 'aggregate': 'SUM'}]", - "Optional 'columns' for cross-tabulation: [{'name': 'region'}]", - ], - error_code="MISSING_PIVOT_FIELDS", - ) - - if not isinstance(config.get("rows", []), list): - return False, ChartGenerationError( - error_type="invalid_rows_format", - message="Rows must be a list of columns", - details="The 'rows' field must be an array of column specifications", - suggestions=[ - "Wrap row columns in array: 'rows': [{'name': 'category'}]", - ], - error_code="INVALID_ROWS_FORMAT", - ) - - if not isinstance(config.get("metrics", []), list): - return False, ChartGenerationError( - error_type="invalid_metrics_format", - message="Metrics must be a list", - details="The 'metrics' field must be an array of metric specifications", - suggestions=[ - "Wrap metrics in array: 'metrics': [{'name': 'sales', " - "'aggregate': 'SUM'}]", - ], - error_code="INVALID_METRICS_FORMAT", - ) - - return True, None - - @staticmethod - def _pre_validate_mixed_timeseries_config( - config: Dict[str, Any], - ) -> Tuple[bool, ChartGenerationError | None]: - """Pre-validate mixed timeseries configuration.""" - missing_fields = [] - - if "x" not in config: - missing_fields.append("'x' (X-axis temporal column)") - if "y" not in config: - missing_fields.append("'y' (primary Y-axis metrics)") - if "y_secondary" not in config: - missing_fields.append("'y_secondary' (secondary Y-axis metrics)") - - if missing_fields: - return False, ChartGenerationError( - error_type="missing_mixed_timeseries_fields", - message=f"Mixed timeseries chart missing required " - f"fields: {', '.join(missing_fields)}", - details="Mixed timeseries charts require an x-axis, primary metrics, " - "and secondary metrics", - suggestions=[ - "Add 'x' field: {'name': 'date_column'}", - "Add 'y' field: [{'name': 'revenue', 'aggregate': 'SUM'}]", - "Add 'y_secondary' field: [{'name': 'orders', " - "'aggregate': 'COUNT'}]", - "Optional: 'primary_kind' and 'secondary_kind' for chart types", - ], - error_code="MISSING_MIXED_TIMESERIES_FIELDS", - ) - - for field_name in ["y", "y_secondary"]: - if not isinstance(config.get(field_name, []), list): - return False, ChartGenerationError( - error_type=f"invalid_{field_name}_format", - message=f"'{field_name}' must be a list of metrics", - details=f"The '{field_name}' field must be an array of metric " - "specifications", - suggestions=[ - f"Wrap in array: '{field_name}': " - "[{'name': 'col', 'aggregate': 'SUM'}]", - ], - error_code=f"INVALID_{field_name.upper()}_FORMAT", - ) - - return True, None - # Per-chart-type error details used by _enhance_validation_error. # Keyed by chart_type discriminator value. + # NOTE: Keep this dict in sync with the plugin registry in + # superset/mcp_service/chart/plugins/ — each registered chart_type must + # have a corresponding entry here so Pydantic parse errors produce + # helpful, type-specific messages. _CHART_TYPE_ERROR_HINTS: Dict[str, Dict[str, Any]] = { "xy": { "error_type": "xy_validation_error",