Skip to content

fix(mcp): accept Superset vocabulary in chart configs and clarify query_dataset metric errors#40972

Draft
richardfogaca wants to merge 1 commit into
apache:masterfrom
richardfogaca:richardfogaca/mcp-chart-config-coercion
Draft

fix(mcp): accept Superset vocabulary in chart configs and clarify query_dataset metric errors#40972
richardfogaca wants to merge 1 commit into
apache:masterfrom
richardfogaca:richardfogaca/mcp-chart-config-coercion

Conversation

@richardfogaca

Copy link
Copy Markdown
Contributor

SUMMARY

LLM clients talking to the MCP service reliably reach for Superset's public vocabulary — datasource_id, viz_type, plugin viz names like echarts_timeseries_bar, form_data fields like show_legend — before consulting the MCP tool schema. Today each of those guesses is rejected by validation, and every rejection costs the client a full model round trip (and, in MCP hosts that gate non-read-only tools, a user approval prompt per retry).

We instrumented live agentic sessions against a workspace running this service and found that every generate_chart retry across all test conditions was the same request-shape guess, in a small set of recurring forms:

  1. datasource_id instead of dataset_id (Superset's REST vocabulary)
  2. viz_type: "bar" / "dist_bar" / "echarts_timeseries_bar" where the config union expects its chart_type discriminator (xy, pie, ...)
  3. form_data field names inside the xy config: show_legend (and a bool where LegendConfig is expected), chart_orientation
  4. bare strings where ColumnRef objects are expected ("x_axis": "genre", "y": ["count"])

A typical session burned 2–4 rejected generate_chart calls plus get_chart_type_schema consultations before converging — roughly doubling wall-clock time for a simple "create a bar chart" request.

Separately, query_dataset's metric validation produced a misleading hint chain: on a dataset with no saved metrics, Unknown metric: 'sum__global_sales' carried no guidance, while the adjacent order_by error suggested Did you mean: global_sales? (a column, valid for order_by) — which LLM clients then tried as a metric, failing again in a loop. The tool is saved-metrics-only by design, but nothing in the error said so.

This PR makes the validation layer absorb the unambiguous synonyms instead of refusing them, and makes the saved-metrics-only contract explicit:

Chart request schemas (chart/schemas.py)

  • ChartRequestNormalizerMixin on GenerateChartRequest, UpdateChartRequest, UpdateChartPreviewRequest, and GenerateExploreLinkRequest:
    • datasource_iddataset_id (explicit dataset_id wins)
    • viz_type key accepted as a chart_type synonym (explicit chart_type wins)
    • common Superset viz plugin names translate to the discriminator + kind: bar/dist_bar/echarts_timeseries_barxy+bar, echarts_timeseries_linexy+line, big_number_totalbig_number, etc. An explicitly provided kind is never overridden.
  • XYChartConfig:
    • x accepts a bare column-name string (matching the existing group_by coercion)
    • y accepts bare strings / a single entry (reuses _normalize_group_by_input)
    • legend gains a show_legend alias and accepts a bool (True{"show": true})
    • orientation gains a chart_orientation alias

The UnknownFieldCheckMixin "did you mean?" behavior is unchanged for genuinely unknown fields — order_desc still gets a clear rejection (covered by a regression test).

query_dataset metric errors (dataset/tool/query_dataset.py)

  • When the dataset has no saved metrics, the error now states the contract and the escape hatch: "This dataset has no saved metrics, and query_dataset accepts saved metric names only (not ad-hoc expressions). Use execute_sql for ad-hoc aggregates, or add a saved metric to the dataset."
  • When metrics exist but nothing fuzzy-matches, the valid metric names are listed (capped at 10) so the client doesn't guess again.
  • The tool docstring states the saved-metrics-only contract up front, since it is the primary text LLM clients read.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (API validation behavior).

Before (live transcript excerpts):

Error: 2 validation errors for call[generate_chart] request.dataset_id
  Field required [input_value={'datasource_id': 23, ...}]
Error: 1 validation error for call[generate_chart] request.config
  Input tag 'bar' found using 'chart_type' does not match any of the
  expected tags: 'xy', 'table', 'pie', ...
Error: 1 validation error for call[generate_chart] request.config.xy
  Value error, Unknown field 'show_legend' — did you mean 'legend'?

After: each of those payloads validates on the first attempt.

TESTING INSTRUCTIONS

pytest tests/unit_tests/mcp_service/chart/test_chart_config_coercion.py
pytest tests/unit_tests/mcp_service/chart/ tests/unit_tests/mcp_service/dataset/

24 new unit tests cover the alias/coercion paths (including precedence rules and the unknown-field regression); the full chart + dataset MCP suites pass (1006 tests).

For a live check: call generate_chart with {"datasource_id": <id>, "config": {"viz_type": "bar", "x_axis": "<column>", "y": [{"name": "<column>", "aggregate": "SUM"}], "show_legend": true}} — it should produce a chart instead of three validation errors.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
  • Introduces new feature or API
  • Removes existing feature or API

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.00000% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.71%. Comparing base (6c5ad1e) to head (4a8a098).

Files with missing lines Patch % Lines
superset/mcp_service/chart/schemas.py 50.00% 20 Missing ⚠️
superset/mcp_service/dataset/tool/query_dataset.py 10.00% 9 Missing ⚠️

❌ Your project check has failed because the head coverage (99.95%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40972      +/-   ##
==========================================
- Coverage   64.30%   63.71%   -0.59%     
==========================================
  Files        2657     2657              
  Lines      144059   144101      +42     
  Branches    33216    33225       +9     
==========================================
- Hits        92639    91817     -822     
- Misses      49798    50660     +862     
- Partials     1622     1624       +2     
Flag Coverage Δ
hive 39.44% <42.00%> (-0.01%) ⬇️
mysql 58.17% <42.00%> (-0.02%) ⬇️
postgres 58.24% <42.00%> (-0.02%) ⬇️
presto 41.03% <42.00%> (-0.01%) ⬇️
python 58.47% <42.00%> (-1.27%) ⬇️
sqlite 57.86% <42.00%> (-0.02%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@richardfogaca richardfogaca force-pushed the richardfogaca/mcp-chart-config-coercion branch from d0f9abb to 4a8a098 Compare June 11, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant