fix(mcp): fall back to form_data spatial query for Deck.gl charts#18
Open
hbrooks wants to merge 6 commits into
Open
fix(mcp): fall back to form_data spatial query for Deck.gl charts#18hbrooks wants to merge 6 commits into
hbrooks wants to merge 6 commits into
Conversation
…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.
…ip columns in Deck.gl fallback - Extract _resolve_deck_gl_metrics to handle point_radius_fixed when type=="metric" (deck_scatter radius, deck_polygon elevation) - Add _deck_gl_null_filters to mirror BaseDeckGLViz.add_null_filters() behavior; applied by default when filter_nulls is not False - Extend resolve_deck_gl_columns to include tooltip_contents column items and cross_filter_column, matching the normal Deck.gl query - Add _deck_gl_tooltip_cols helper for tooltip column extraction - Add unit tests covering all three behaviors
…k_geojson _resolve_deck_gl_metrics now: - Returns [] immediately for deck_geojson, matching DeckGeoJson.query_obj() which explicitly forces metrics=[] regardless of form_data - Only includes size/metric values that are dicts (metric references); scalar values like "100" in deck_geojson/deck_path fixtures are fixed display settings and must not be passed as query metrics Adds viz_type param and passes it from build_query_dicts_from_form_data. Adds regression tests using scalar size fixtures.
…Deck.gl fallback - Add _is_metric_ref() to distinguish saved metric keys (non-numeric strings like "count") from fixed display settings (numeric strings like "100"); both size and metric fields are now validated with this helper so string metric references are no longer silently dropped - Extend _deck_gl_null_filters() to add IS NOT NULL for the geojson column, mirroring the behavior of DeckGeoJson when filter_nulls is enabled - Add unit tests for _is_metric_ref and the new integration paths
…fallback
Legacy deck_scatter charts store point_radius_fixed as a plain metric key
string (e.g. "count") rather than the dict form {"type":"metric","value":...}.
The frontend isMetricValue() treats any non-numeric string as a valid metric
ref, so the fallback query builder now does the same via _is_metric_ref.
…icts _is_metric_ref returns True for any dict, so the elif branch added in e32a683 would incorrectly include {"type":"fix","value":100} (a fixed pixel radius) as a SQL metric. Add isinstance(prf, str) guard so the legacy bare-string path only fires for actual string values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of apache/superset#40339 by @aminghadersohi