Skip to content

fix(mcp): fall back to form_data spatial query for Deck.gl charts#40339

Open
aminghadersohi wants to merge 1 commit into
apache:masterfrom
aminghadersohi:oss-104290
Open

fix(mcp): fall back to form_data spatial query for Deck.gl charts#40339
aminghadersohi wants to merge 1 commit into
apache:masterfrom
aminghadersohi:oss-104290

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

SUMMARY

get_chart_data MCP tool was returning a MissingQueryContext error for
Deck.gl charts (deck_scatter, deck_arc, deck_hex, etc.) when those charts had
no saved query_context. Instead of offering a data retrieval path, the tool
told users to re-open and re-save the chart in Superset.

Deck.gl charts use spatial column configs (lat/lon pairs, geohash, delimited
coordinates, GeoJSON) rather than the standard metrics/groupby structure.
This PR teaches the form_data fallback path how to extract those columns.

Changes:

  • Add _deck_gl_spatial_cols (private helper) and resolve_deck_gl_columns
    to chart_helpers.py. The helper reads spatial, start_spatial,
    end_spatial, line_column, geojson, dimension, and js_columns
    fields from form_data and returns the SQL column names needed by the
    underlying query — mirroring BaseDeckGLViz.query_obj() semantics.

  • Route deck_* viz types through the new helpers inside
    build_query_dicts_from_form_data. When a size/metric field is present
    (e.g., point-radius or weight metric), it is included in the query; spatial
    columns become the groupby dimensions. Without a metric, spatial columns
    become raw-select columns.

  • Remove the hard error block in get_chart_data.py that rejected all
    Deck.gl charts with MissingQueryContext. The existing safety-net (empty
    columns + empty metrics) still covers charts whose spatial config is
    completely missing.

  • Add unit tests covering: latlong, delimited, geohash spatial types;
    arc start/end spatial; line_column; geojson; dimension;
    js_columns; column deduplication; the build_query_dicts_from_form_data
    Deck.gl branch with and without metrics.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (backend-only change to MCP tool data retrieval)

TESTING INSTRUCTIONS

  1. Find or create a Deck.gl chart (e.g., Scatter, Arc, Hex) in Superset
    whose params JSON has a spatial field but no saved
    query_context.
  2. Call the get_chart_data MCP tool with that chart's ID.
  3. Before: ChartError with error_type: "MissingQueryContext" and a
    message asking the user to re-save the chart.
  4. After: The tool queries the underlying dataset using the lat/lon (or
    geohash/delimited) columns from form_data and returns tabular data.

Run the new unit tests:

pytest tests/unit_tests/mcp_service/chart/test_chart_helpers.py -x -q \
  -k "deck"

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

…t_data

Deck.gl chart types (deck_scatter, deck_arc, deck_hex, etc.) use spatial
column configs (lat/lon pairs, geohash, delimited coordinates) rather than
the standard metrics/groupby structure. The get_chart_data MCP tool was
returning an early MissingQueryContext error for these charts when no
saved query_context was present, instead of falling back to form_data.

- Add _deck_gl_spatial_cols and resolve_deck_gl_columns helpers to
  chart_helpers.py to extract SQL column names from spatial control configs
  (latlong, delimited, geohash) plus line_column, geojson, dimension, and
  js_columns fields.
- Route deck_ viz types through the new helpers inside
  build_query_dicts_from_form_data, producing a raw-column or
  grouped-metric query matching BaseDeckGLViz.query_obj() semantics.
- Remove the early-exit error block for deck_ charts in get_chart_data.py;
  the existing safety-net (empty columns + empty metrics) still guards
  charts whose spatial config is completely absent.
- Add unit tests covering latlong, delimited, geohash, arc start/end,
  line_column, geojson, dimension, js_columns, deduplication, and the
  build_query_dicts_from_form_data Deck.gl branch.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 6.89655% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.11%. Comparing base (d203f0d) to head (ed15be8).
⚠️ Report is 28 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/chart/chart_helpers.py 6.89% 27 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40339      +/-   ##
==========================================
- Coverage   64.18%   64.11%   -0.08%     
==========================================
  Files        2591     2592       +1     
  Lines      138459   138930     +471     
  Branches    32118    32223     +105     
==========================================
+ Hits        88875    89069     +194     
- Misses      48052    48329     +277     
  Partials     1532     1532              
Flag Coverage Δ
hive 39.28% <6.89%> (-0.13%) ⬇️
mysql 58.78% <6.89%> (-0.28%) ⬇️
postgres 58.87% <6.89%> (-0.28%) ⬇️
presto 40.95% <6.89%> (-0.15%) ⬇️
python 60.42% <6.89%> (-0.15%) ⬇️
sqlite 58.51% <6.89%> (-0.28%) ⬇️
unit 100.00% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 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.

@aminghadersohi aminghadersohi marked this pull request as ready for review May 22, 2026 03:08
@dosubot dosubot Bot added the viz:charts:deck.gl Related to deck.gl charts label May 22, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 23, 2026

Code Review Agent Run #7b748a

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset/mcp_service/chart/tool/get_chart_data.py - 1
    • Missing test for deck.gl fallback · Line 340-367
      The deleted deck.gl special-case block lacks test coverage. While the removal is correct (build_query_dicts_from_form_data handles deck_* viz types via resolve_deck_gl_columns), adding a test ensures the fallback path works for deck.gl charts without query_context.
Review Details
  • Files reviewed - 3 · Commit Range: ed15be8..ed15be8
    • superset/mcp_service/chart/chart_helpers.py
    • superset/mcp_service/chart/tool/get_chart_data.py
    • tests/unit_tests/mcp_service/chart/test_chart_helpers.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L viz:charts:deck.gl Related to deck.gl charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant