richardfogaca opened a new pull request, #40099:
URL: https://github.com/apache/superset/pull/40099
### SUMMARY
`get_chart_data` and `get_chart_preview` can be called with an Explore
`form_data_key` so MCP consumers inspect the same unsaved chart state a user
sees in Explore. The previous direct QueryObject construction copied only the
already-concrete `filters` field, so cached `adhoc_filters` were left out and
chart data/preview could return unfiltered rows.
This change:
- adds a shared MCP helper that normalizes cached/direct chart `form_data`
before QueryContext construction;
- preserves sibling legacy `filters`, `where`, and `having` when
`adhoc_filters` are also present;
- applies the same normalized filter fields to `get_chart_data`,
`get_chart_preview`, and the adjacent `get_chart_sql` fallback path;
- keeps the behavior query-side by reusing Superset's existing filter
merge/split helpers instead of post-filtering returned rows.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - backend MCP chart retrieval change with no UI rendering changes.
Before the fix, a cached table chart state with `adhoc_filters: gender ==
boy` could return both `girl` and `boy` rows through MCP data/preview calls.
After the fix, the same request shape returns only the filtered `boy` row, and
preview output excludes `girl`.
### TESTING INSTRUCTIONS
- [x] `pytest tests/unit_tests/mcp_service/chart/test_chart_helpers.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py`
Confirms the shared normalization helper and affected MCP chart tool query
payloads apply cached adhoc filters and preserve legacy-plus-adhoc filter state.
- [x] `python -m py_compile superset/mcp_service/chart/chart_helpers.py
superset/mcp_service/chart/tool/get_chart_data.py
superset/mcp_service/chart/tool/get_chart_preview.py
superset/mcp_service/chart/tool/get_chart_sql.py
tests/unit_tests/mcp_service/chart/test_chart_helpers.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py`
Confirms the changed backend modules and focused tests compile.
- [x] `ruff check superset/mcp_service/chart/chart_helpers.py
superset/mcp_service/chart/tool/get_chart_data.py
superset/mcp_service/chart/tool/get_chart_preview.py
superset/mcp_service/chart/tool/get_chart_sql.py
tests/unit_tests/mcp_service/chart/test_chart_helpers.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py`
- [x] `ruff format --check superset/mcp_service/chart/chart_helpers.py
superset/mcp_service/chart/tool/get_chart_data.py
superset/mcp_service/chart/tool/get_chart_preview.py
superset/mcp_service/chart/tool/get_chart_sql.py
tests/unit_tests/mcp_service/chart/test_chart_helpers.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py`
- [x] `pre-commit run --files superset/mcp_service/chart/chart_helpers.py
superset/mcp_service/chart/tool/get_chart_data.py
superset/mcp_service/chart/tool/get_chart_preview.py
superset/mcp_service/chart/tool/get_chart_sql.py
tests/unit_tests/mcp_service/chart/test_chart_helpers.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py`
- [x] Live MCP/API validation in a local Superset compose runtime: created
cached Explore table `form_data` for the `birth_names` dataset with
`adhoc_filters: gender == boy`, then called MCP `get_chart_info`,
`get_chart_data`, and `get_chart_preview` through the chart tool entry points.
The observed payload showed the cached filter was present, `get_chart_data`
returned only `boy`, and table preview did not contain `girl`; before the fix
the same scenario returned both `girl` and `boy`.
- [x] Live MCP/API adversarial validation in the same runtime: combined
cached `gender == boy` with request `extra_form_data` `gender == girl`, and
also combined cached legacy `filters: gender == boy` with cached
`adhoc_filters: gender == girl`. The observed data/preview row counts were zero
for contradictory filters, and `get_chart_sql` produced SQL containing both
filter predicates, confirming the normalization merges filter sources instead
of replacing one with another.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: no public issue
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Reviewer focus: `chart_helpers.py` is the ownership boundary for MCP
form-data normalization. The tool modules should only build queries from the
helper's normalized output, so data, preview, and SQL stay aligned.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
