Skip to content

MCP: nested chart-config models silently accept unknown fields — update_chart reports success while discarding them #42626

Description

@DanRoscigno

Bug

superset/mcp_service/chart/schemas.py defines UnknownFieldCheckMixin to stop unknown fields from being silently dropped:

def _check_unknown_fields(data: Any, model_class: type[BaseModel]) -> Any:
    """Raise ValueError for unrecognized fields with 'did you mean?' suggestions.

    Catches fields that would be silently dropped by extra='ignore' and provides
    actionable error messages to help LLMs self-correct parameter names.
    """

All ten top-level chart config models inherit it — PieChartConfig, PivotTableChartConfig, MixedTimeseriesChartConfig, HandlebarsChartConfig, BigNumberChartConfig, TableChartConfig, XYChartConfig, HistogramChartConfig, BoxPlotChartConfig, WaterfallChartConfig.

The nested models those configs are composed of do not:

Model Line (master @ 7b351d5) Base
ColumnRef 774 BaseModel
AxisConfig 895 BaseModel
LegendConfig 901 BaseModel
CurrencyFormat 906 BaseModel
FilterConfig 929 BaseModel
SortByConfig 997 BaseModel

So pydantic's default extra="ignore" applies one level down, and unknown-field protection is enforced at exactly one level of nesting.

Reproduction

from superset.mcp_service.chart.schemas import parse_chart_config

base = {"chart_type": "xy", "kind": "bar",
        "x": {"name": "state"}, "y": [{"name": "orders", "aggregate": "SUM"}]}

# unknown field NESTED inside x_axis
cfg = parse_chart_config({**base, "x_axis": {"title": "State", "sort_by": "metric"}})
print(cfg.x_axis)

# the same unknown field at TOP level
parse_chart_config({**base, "sort_by": "metric"})

Output:

AxisConfig(title='State', scale='linear', format=None)     # sort_by silently gone
ValueError: 1 validation error for tagged-union[...]
xy
  Value error, Unknown field 'sort_by'. Valid fields: breakdown, chart_type, ...

Same asymmetry through the MCP tools. Against a saved bar chart, update_chart with an unknown field nested in x_axis, x, or legend returns success: true and a chart URL:

{"chart": {"id": 1, "slice_name": "Orders by Customer State", ...},
 "success": true, "error": null, "warnings": []}

while a subsequent get_chart_info shows the stored form_data unchanged:

"x_axis_sort_series_type": "name", "x_axis_sort_series_ascending": true

The same field at the top level of config is correctly rejected with the "Valid fields:" list.

Impact

The mixin exists specifically so that LLM clients get an actionable error instead of a silent drop. Nested objects are where axis, legend, currency and filter options live — precisely the options a client is most likely to guess at, and the ones least likely to be memorised correctly.

The failure mode is worse than a plain rejection. The tool returns success: true with a chart URL and no warnings, so the caller reasonably concludes the setting was applied. Detecting otherwise requires a follow-up get_chart_info and knowing which native form_data key the config field maps to. An agent that self-corrects on error has nothing to correct against, so it moves on believing the chart is configured.

Encountered while trying to sort a bar chart by value via x_axis: {"sort_by": "metric"}: three successive update_chart calls each returned success and changed nothing.

Suggested fix

Have the nested models inherit UnknownFieldCheckMixin instead of BaseModel:

class AxisConfig(UnknownFieldCheckMixin):
    ...

_check_unknown_fields is already generic over model_class and resolves aliases via _get_known_fields, so no other change is required. Models using validation_alias (ColumnRef, FilterConfig, SortByConfig) keep working, since _get_known_fields already collects AliasChoices.

One consideration for reviewers: this makes previously-accepted payloads fail. That is the intent of the mixin, and it matches the existing top-level behaviour, but it is a behaviour change for any client currently passing extra nested keys.

I'm happy to open a PR with the change plus a regression test in tests/unit_tests/mcp_service/chart/test_chart_schemas.py.

Environment

  • Apache Superset 6.1.0 (docker), pydantic 2.11.7, fastmcp 3.4.5, Python 3.10.20
  • Class definitions confirmed unchanged on master @ 7b351d5

Metadata

Metadata

Assignees

No one assigned

    Labels

    change:backendRequires changing the backendvalidation:requiredA committer should validate the issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions