Repository navigation
fix(mcp): carry semantic selection versions for new charts - #45828
mikebridge wants to merge 4 commits into
Conversation
Stamp only newly generated chart and Explore selections, forward their identity through query extras without certifying external filters, and apply semantic temporal metadata before sorting. Keep existing-chart updates unstamped pending a complete-reselection contract.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #45828 +/- ##
===========================================
- Coverage 85.00% 65.24% -19.76%
===========================================
Files 3080 3080
Lines 209252 210521 +1269
Branches 49292 49505 +213
===========================================
- Hits 177881 137363 -40518
- Misses 27990 69981 +41991
+ Partials 3381 3177 -204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
The external-selection guard still has a fail-open merge path.
Keep each contributor's source status before dashboard and request filters lose their origin. Apply the same guard when appending filters to a saved query context so missing or stale provenance cannot borrow its version. Addresses review thread 4428377061.
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
gabotorresruiz
left a comment
gabotorresruiz
left a comment
There was a problem hiding this comment.
@mikebridge thanks for this! LGTM. New selections are only stamped after every metric, dimension, filter and sort reference has passed the view checks, so a stale ID is refused before anything is saved or queried, and the merge paths keep unproven inputs unverified. I left one non-blocking note on the cross viz update_chart path.
I ran the MCP, semantic layer and common unit suites on this branch plus a few stale ID probes through generate_chart and generate_explore_link. CI is green.
| # The existing config merge may drop the old marker; neither retaining | ||
| # nor dropping it certifies these selections as current. | ||
| assert form_data.get("semantic_selection_version") in (None, saved_version) |
There was a problem hiding this comment.
Not a blocker, more of a follow-up. The drop happens on cross viz updates: _merge_allowlisted_form_data only carries keys from FORM_DATA_UPDATE_PRESERVE_KEYS, and semantic_selection_version isn't one of them. So on this branch, a chart that generate_chart just stamped followed by update_chart changing kind from bar to line builds its compile query with no version and comes back with "Saved semantic selections use an older identity format", while bar to bar keeps the version and saves fine.
Keeping whatever marker the saved chart already has never upgrades an older one, so could it ride along with the envelope keys?
"envelope": frozenset(
{
...
"semantic_selection_version",
...
}
),A case here with a current saved_version and a viz change would pin it. Or am I missing a reason cross viz updates should drop it?
There was a problem hiding this comment.
Good catch, fixed here rather than in a follow-up: 8441c48 carries semantic_selection_version along with the envelope keys on cross-viz updates, keeping whatever marker the saved chart already has (an older one is never upgraded). A generate then bar-to-line update now keeps the version and saves.
Carry the saved selection version and its source evidence through the cross-visualization envelope merge. Dropping the version refuses newly generated charts, while retaining it without source evidence can lose external-selection validation. Cover generated bar-to-line saves, preview/save marker preservation, and missing or older versions without upgrading them.
github-actions
Bot
added
the
requires:rebase
SUMMARY
TL;DR: MCP chart tools can now save charts and build Explore links on versioned semantic views.
Background: Semantic views that version their member identifiers (for example, table-qualified Snowflake names such as
O.TOTAL_REVENUE) expose asemantic_selection_version. Explore saves it with a chart's form data and sends it in each query's extras, and the query layer refuses selections whose version is missing or old, so a chart saved under an older naming scheme can't silently resolve to the wrong member.Problem:
generate_chartandgenerate_explore_linkbuilt semantic-view form data without the version, so every new MCP chart on a versioned view looked like an old chart and was refused ("Saved semantic selections use an older identity format…"). The MCP query builder also dropped a top-level version instead of forwarding it to query extras. Separately, an XY chart with a categorical semantic x-axis was first mapped as temporal, which ignored the requested sort and produced a false "x-axis is temporal" warning.What this changes:
generate_chartandgenerate_explore_linkare stamped with the view's current selection version.update_chartis unchanged: rename, add-columns and config updates on existing charts don't receive a new version, because they can retain selections from the saved chart. Safely re-stamping existing charts needs a separate full-reselection rule.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: MCP request and payload changes only.
TESTING INSTRUCTIONS
generate_chartwithview_id, current metric and dimension IDs andsave_chart=true(for example a bar chart of a revenue metric by a segment dimension). It saves, and the saved params carrysemantic_selection_version.generate_explore_linkfor the same view: the form data carries the version and the chart renders.Automated: MCP chart and Explore tool tests, including generation, Explore links, query extras for every query, wrong/old/absent provenance, and existing-chart updates staying unstamped.
ADDITIONAL INFORMATION
No migration, provider-version or frontend change.