mirror of
https://github.com/apache/superset.git
synced 2026-05-21 15:55:10 +00:00
refactor(mcp): eliminate dead code and complete plugin registry dispatch
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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 []
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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: '<ul>{{#each data}}<li>{{this.name}}: "
|
||||
"{{this.value}}</li>{{/each}}</ul>'",
|
||||
],
|
||||
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: '<ul>{{#each data}}<li>{{this.name}}</li>{{/each}}</ul>'",
|
||||
],
|
||||
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",
|
||||
|
||||
Reference in New Issue
Block a user