Skip to content

fix(mcp): clear stale query_context in update_chart so filters and row_limit are applied#39413

Open
gabotorresruiz wants to merge 2 commits intoapache:masterfrom
gabotorresruiz:fix/mcp-filters-row-limit-ignored
Open

fix(mcp): clear stale query_context in update_chart so filters and row_limit are applied#39413
gabotorresruiz wants to merge 2 commits intoapache:masterfrom
gabotorresruiz:fix/mcp-filters-row-limit-ignored

Conversation

@gabotorresruiz
Copy link
Copy Markdown
Contributor

@gabotorresruiz gabotorresruiz commented Apr 16, 2026

SUMMARY

update_chart writes to params but never clears query_context. Since get_chart_data reads query_context first when present, filters and row_limit set via MCP are silently ignored on any chart previously opened in Explore.

Sets "query_context": None on config updates. Also returns form_data in the response and fixes a misleading docstring.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  1. Open a saved chart in Explore (populates query_context)
  2. Call update_chart with generate_preview=False, a filter, and a row_limit
  3. Call get_chart_data -- data should now reflect the update

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

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 16, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 9828dd2
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69e133571f650a00079ff57e
😎 Deploy Preview https://deploy-preview-39413--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@gabotorresruiz gabotorresruiz force-pushed the fix/mcp-filters-row-limit-ignored branch from 06a0f1e to 9828dd2 Compare April 16, 2026 19:07
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.49%. Comparing base (69f062b) to head (d6deaff).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/chart/tool/update_chart.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39413      +/-   ##
==========================================
- Coverage   64.51%   64.49%   -0.02%     
==========================================
  Files        2557     2557              
  Lines      133049   133092      +43     
  Branches    30901    30909       +8     
==========================================
+ Hits        85834    85837       +3     
- Misses      45725    45765      +40     
  Partials     1490     1490              
Flag Coverage Δ
hive 39.87% <0.00%> (-0.03%) ⬇️
mysql 60.49% <0.00%> (-0.04%) ⬇️
postgres 60.57% <0.00%> (-0.04%) ⬇️
presto 41.66% <0.00%> (-0.03%) ⬇️
python 62.15% <0.00%> (-0.05%) ⬇️
sqlite 60.20% <0.00%> (-0.04%) ⬇️
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.

@gabotorresruiz gabotorresruiz force-pushed the fix/mcp-filters-row-limit-ignored branch 2 times, most recently from ca3fa9d to 7ba2be7 Compare April 16, 2026 19:47
@gabotorresruiz gabotorresruiz force-pushed the fix/mcp-filters-row-limit-ignored branch from 7ba2be7 to 3d15e17 Compare April 16, 2026 20:46
@gabotorresruiz gabotorresruiz marked this pull request as ready for review April 16, 2026 21:02
@dosubot dosubot bot added the change:backend Requires changing the backend label Apr 16, 2026
@aminghadersohi
Copy link
Copy Markdown
Contributor

Two concerns worth a look:

  1. 559 lines of new tests for a small production fix — heavy mocking, verify it's all necessary
  2. _compile_chart() runs on every persist — adds latency + could fail valid configs; confirm it's needed given existing validation

Comment thread superset/mcp_service/chart/tool/update_chart.py
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Apr 16, 2026

Code Review Agent Run #3a2db7

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 3d15e17..1a7b392
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/chart/tool/update_chart.py
    • tests/unit_tests/mcp_service/chart/tool/test_update_chart.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

@gabotorresruiz
Copy link
Copy Markdown
Contributor Author

Hey @aminghadersohi, I’ve fixed those issues. There were some leftovers from a merge conflict. Everything should be good now.

@gabotorresruiz gabotorresruiz force-pushed the fix/mcp-filters-row-limit-ignored branch from 1a7b392 to d6deaff Compare April 17, 2026 15:54
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Apr 17, 2026

Code Review Agent Run #5fdd44

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 3d15e17..d6deaff
    • superset/mcp_service/chart/tool/update_chart.py
    • tests/unit_tests/mcp_service/chart/tool/test_update_chart.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

change:backend Requires changing the backend size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants