Skip to content

fix(mcp): expose query context to Jinja macros - #42822

Draft
aminghadersohi wants to merge 3 commits into
apache:masterfrom
aminghadersohi:aminghadersohi/mcp-jinja-form-data
Draft

fix(mcp): expose query context to Jinja macros#42822
aminghadersohi wants to merge 3 commits into
apache:masterfrom
aminghadersohi:aminghadersohi/mcp-jinja-form-data

Conversation

@aminghadersohi

Copy link
Copy Markdown
Contributor

SUMMARY

The chart data API makes its query request available through Flask's global form-data fallback because Jinja macros such as filter_values, get_filters, and url_param read query inputs from that context. Programmatically constructed MCP queries skipped that step, so virtual datasets and metrics using those macros could fail despite the equivalent API query succeeding.

This extracts the API's form-data assignment into a shared helper and uses it for get_chart_data and query_dataset. MCP query contexts are represented in the same datasource/queries/form-data shape consumed by the existing Jinja lookup path; no synthetic request context or duplicated macro-specific behavior is introduced.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; this is a backend-only query execution fix.

TESTING INSTRUCTIONS

  • pytest -q tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py (136 passed)
  • pytest -q tests/unit_tests/mcp_service (3318 passed; one unrelated existing health-check smoke failure because the test environment has no version string configured)
  • uvx pre-commit run --files superset/charts/data/api.py superset/charts/data/form_data.py superset/mcp_service/chart/tool/get_chart_data.py superset/mcp_service/dataset/tool/query_dataset.py tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
  • pre-commit run --all-files (Python checks passed; frontend-only hooks could not complete because frontend dependencies such as glob and postcss-styled-syntax are not installed; the formatter also reported pre-existing frontend formatting drift)

The regression tests execute filter_values and get_filters during an MCP query_dataset call and cover url_param plus implicit dataset lookup for metric() using chart-shaped query context.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

Blast radius

This affects only MCP chart and dataset query execution plus the existing chart-data API context assignment. It does not change authorization, RLS, workspace isolation, schemas, migrations, feature flags, or non-MCP query behavior.

Risk & rollback

Risk is limited to the Flask-global form-data payload used while compiling MCP queries. The payload is built from the already validated query context and matches the existing API representation. Rollback is a revert of this commit.

Review guidance

Start with superset/charts/data/form_data.py, then verify the two MCP call sites populate it immediately before ChartDataCommand execution. The tests document the request-dependent macro boundary.

Eval evidence

No model or prompt evaluation was run because this change does not modify tool schemas, descriptions, prompts, model routing, or generated responses. The deterministic MCP regression coverage passes 136/136; the full MCP unit suite passes 3318/3319 with the environment-only health-check failure described above.

Cost & latency delta

No model parameters, prompts, or tool payload schemas changed, so token and model-cost delta is zero. The added work serializes the already-built query objects once per affected call; no external request or database query is added.

Prompt / non-determinism

No prompt, model routing, or non-deterministic behavior changed.

@github-actions github-actions Bot added the api Related to the REST API label Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.73%. Comparing base (4278f4b) to head (bca3fba).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/chart/tool/get_chart_data.py 0.00% 2 Missing ⚠️
superset/mcp_service/dataset/tool/query_dataset.py 0.00% 2 Missing ⚠️
superset/charts/data/api.py 75.00% 1 Missing ⚠️
superset/charts/data/form_data.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42822      +/-   ##
==========================================
- Coverage   65.73%   65.73%   -0.01%     
==========================================
  Files        2843     2844       +1     
  Lines      162653   162665      +12     
  Branches    37239    37239              
==========================================
+ Hits       106919   106923       +4     
- Misses      53642    53649       +7     
- Partials     2092     2093       +1     
Flag Coverage Δ
hive 37.96% <46.66%> (+<0.01%) ⬆️
mysql 57.79% <60.00%> (+<0.01%) ⬆️
postgres 57.83% <60.00%> (-0.01%) ⬇️
presto 39.87% <60.00%> (+<0.01%) ⬆️
python 59.20% <60.00%> (-0.01%) ⬇️
sqlite 57.47% <60.00%> (+<0.01%) ⬆️
unit 100.00% <ø> (ø)

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.

@aminghadersohi
aminghadersohi force-pushed the aminghadersohi/mcp-jinja-form-data branch from 993cf12 to f96ad00 Compare August 6, 2026 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API review:draft size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants