fix(mcp): use chart.query_context for get_chart_data like the API does #36937

aminghadersohi · 2026-01-07T04:32:00Z

Summary

Fixed ctx error: Added missing @parse_request(GetChartDataRequest) decorator to get_chart_data tool, which was causing the error: missing 1 required positional argument: 'ctx' when calling from MCP clients
Fixed single row data issue: Changed implementation to use chart.query_context (same approach as /api/v1/chart/{id}/data endpoint) instead of manually constructing queries, which was causing temporal line charts to return a single aggregated value instead of all data points
Respects chart's configured limits: Trusts chart's configured row_limit from form_data (which respects global ROW_LIMIT and SQL_MAX_ROW config). Removed MAX_ROW_LIMIT constant per review feedback - charts already have enforced row limits from global config.

Test Plan

Verified Fix with Temporal Line Chart

Created a temporal line chart (chart 135) with order_date vs SUM(sales) on cleaned_sales_data dataset and compared API vs MCP responses:

Aspect	API `/api/v1/chart/135/data/`	MCP `get_chart_data`
Row count	252	252 ✅
Columns	`order_date`, `SUM(sales)`	`order_date`, `SUM(sales)` ✅
First data point	`2003-01-06, $12,133.25`	`2003-01-06, $12,133.25` ✅
Last data point	`2005-05-31, $78,918.03`	`2005-05-31, $78,918.03` ✅

API Response (first 5 rows):

{"order_date": 1041811200000.0, "SUM(sales)": 12133.25}
{"order_date": 1042070400000.0, "SUM(sales)": 11432.34}
{"order_date": 1042156800000.0, "SUM(sales)": 6864.05}
{"order_date": 1043798400000.0, "SUM(sales)": 54702.0}
{"order_date": 1043971200000.0, "SUM(sales)": 44621.96}

MCP Response (first 5 rows):

{"order_date": "2003-01-06T00:00:00", "SUM(sales)": 12133.25}
{"order_date": "2003-01-09T00:00:00", "SUM(sales)": 11432.34}
{"order_date": "2003-01-10T00:00:00", "SUM(sales)": 6864.05}
{"order_date": "2003-01-29T00:00:00", "SUM(sales)": 54702}
{"order_date": "2003-01-31T00:00:00", "SUM(sales)": 44621.96}

Result: Both return identical data - 252 temporal data points with matching values.

Tested Fallback Path (charts without saved `query_context`)

Chart	Type	Rows Returned	Status
Chart 2: "Most Populated Countries"	table	214 rows	✅
Chart 3: "Growth Rate"	timeseries line	214 rows	✅
Chart 7: "World's Pop Growth"	area	7 rows	✅
Chart 12: "Genders"	pie	2 rows	✅
Chart 13: "Trends"	timeseries line	250 rows	✅

All charts return the correct number of data points instead of a single aggregated value.

Additional Information

Reported Issues (by @villebro)

get_chart_data was returning: missing 1 required positional argument: 'ctx'
Temporal line charts returning single aggregated value (e.g., 7291) instead of all data points

Root Causes

The @parse_request decorator was missing - this decorator handles FastMCP's context injection
The original implementation manually constructed queries from form_data parts instead of using the chart's saved query_context

Key Fix

Now uses ChartDataQueryContextSchema().load(query_context_json) - the same approach as the /api/v1/chart/{id}/data endpoint:

if chart.query_context:
    query_context_json = utils_json.loads(chart.query_context)
    query_context = ChartDataQueryContextSchema().load(query_context_json)

🤖 Generated with Claude Code

…add raw data option This PR fixes two issues with the MCP get_chart_data tool: 1. Missing @parse_request decorator causing "missing 1 required positional argument: 'ctx'" error when calling the tool from MCP clients 2. Charts returning only a single aggregated row instead of full data. Added `include_raw_data` option that retrieves all columns from the underlying dataset without aggregation, useful for charts like big_number. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

codeant-ai-for-open-source · 2026-01-07T04:32:04Z

CodeAnt AI is reviewing your PR.

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

bito-code-review · 2026-01-07T04:32:09Z

Code Review Agent Run #047a2b

Actionable Suggestions - 0

Additional Suggestions - 1

superset/mcp_service/chart/schemas.py - 1
- Missing test coverage for new feature · Line 978-985
  
  The new include_raw_data field adds functionality to retrieve raw dataset data instead of aggregated chart results, but there are no unit tests covering this behavior. This increases regression risk for the new feature. Please add comprehensive tests for the raw data mode.

Review Details

Files reviewed - 2 · Commit Range: d73b5f8..d73b5f8
- superset/mcp_service/chart/schemas.py
- superset/mcp_service/chart/tool/get_chart_data.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 [email protected].

Documentation & Help

AI Code Review powered by

codeant-ai-for-open-source · 2026-01-07T04:34:39Z

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review Unbounded row fetch risk When `include_raw_data` is true the code uses `request.limit or 100` as the `row_limit`. If clients pass a very large `limit` (or none is provided) this may return large datasets and cause heavy DB load / memory use. Consider enforcing a configurable hard cap or validating the requested limit. Backward-incompatible schema change The `DataColumn` model now requires `sample_values`, `null_count`, and `unique_count` (no defaults). Making these fields required can cause validation errors where code paths / tests or existing responses construct chart data without those fields. This may break consumers that expect the previous, lighter-weight schema.

netlify · 2026-01-07T04:34:52Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`d73b5f8`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/695de1c3415e5600088ea250
😎 Deploy Preview	https://deploy-preview-36937--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

superset/mcp_service/chart/tool/get_chart_data.py

codeant-ai-for-open-source · 2026-01-07T04:35:54Z

CodeAnt AI finished reviewing your PR.

codecov · 2026-01-07T04:44:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.18%. Comparing base (9aff89c) to head (2f74de9).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #36937       +/-   ##
===========================================
+ Coverage        0   68.18%   +68.18%     
===========================================
  Files           0      639      +639     
  Lines           0    47701    +47701     
  Branches        0     5210     +5210     
===========================================
+ Hits            0    32527    +32527     
- Misses          0    13893    +13893     
- Partials        0     1281     +1281

Flag	Coverage Δ
hive	`43.08% <ø> (?)`
mysql	`66.19% <ø> (?)`
postgres	`66.24% <ø> (?)`
presto	`46.67% <ø> (?)`
python	`68.15% <ø> (?)`
sqlite	`65.96% <ø> (?)`
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.

Addresses reviewer feedback: - Add MAX_ROW_LIMIT (10000) to prevent excessive memory/DB load when requesting large amounts of data - Add comprehensive unit tests for GetChartDataRequest schema including the new include_raw_data field and row limit capping logic 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

aminghadersohi · 2026-01-07T04:52:19Z

Addressing Review Feedback

✅ Fixed: Unbounded row fetch risk

Added MAX_ROW_LIMIT = 10000 constant and capped all row limit calculations using min(request.limit or 100, MAX_ROW_LIMIT). This prevents excessive memory/DB load when clients request large datasets.

✅ Added: Unit tests for new feature

Added comprehensive unit tests in tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py covering:

GetChartDataRequest schema validation including the new include_raw_data field
MAX_ROW_LIMIT constant and row limit capping logic
All 13 tests pass

ℹ️ Re: Backward-incompatible schema change (False Positive)

The CodeAnt review flagged DataColumn model fields as potentially backward-incompatible, but this is not related to this PR. The DataColumn schema was not modified in this PR - it's pre-existing code. My changes only touched:

GetChartDataRequest schema - added new optional field include_raw_data with default False (backward compatible)
get_chart_data function - added @parse_request decorator and raw data logic

The flagged lines (R1117-R1130) are far outside my diff which only modified lines 978-985 in schemas.py.

bito-code-review · 2026-01-07T05:56:16Z

Code Review Agent Run #87d875

Actionable Suggestions - 0

Additional Suggestions - 1

tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py - 1

Use specific exception type · Line 96-96

While ValueError works since ValidationError inherits from it, using ValidationError directly improves clarity and specificity for Pydantic validation errors in this test.

Code suggestion

--- a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
+++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
 @@ -22,6 +22,7 @@
  import pytest
 
+from pydantic import ValidationError
  from superset.mcp_service.chart.schemas import GetChartDataRequest
  from superset.mcp_service.chart.tool.get_chart_data import MAX_ROW_LIMIT
 
 @@ -93,7 +94,7 @@
      def test_invalid_format(self):
          """Test that invalid format raises validation error."""
          with pytest.raises(
-            ValueError, match="Input should be 'json', 'csv' or 'excel'"
+            ValidationError, match="Input should be 'json', 'csv' or 'excel'"
          ):
              GetChartDataRequest(identifier=1, format="invalid")

Review Details

Files reviewed - 3 · Commit Range: d73b5f8..d83b539
- superset/mcp_service/chart/schemas.py
- superset/mcp_service/chart/tool/get_chart_data.py
- tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.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 [email protected].

Documentation & Help

AI Code Review powered by

This addresses Vitor's feedback that get_chart_data was returning incorrect data (e.g., a single aggregated value instead of all data points). The root cause was that the code was manually constructing a query from form_data parts (groupby, metrics), which doesn't produce the same results as the chart visualization. The fix now: 1. Uses chart.query_context (the saved query context) which contains all the information needed to reproduce the chart's data exactly as shown in the visualization - same approach as /api/v1/chart/{id}/data endpoint 2. Falls back to form_data construction only if query_context is unavailable 3. Removed include_raw_data option (was solving the wrong problem) 4. Updated tests accordingly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

codeant-ai-for-open-source · 2026-01-07T14:26:30Z

CodeAnt AI is running Incremental review

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

superset/mcp_service/chart/tool/get_chart_data.py

Vitor-Avila

Left a non-blocking comment

bito-code-review · 2026-01-07T15:28:39Z

Code Review Agent Run #9e29fa

Actionable Suggestions - 0

Additional Suggestions - 1

superset/mcp_service/chart/tool/get_chart_data.py - 1
- Missing error handling for schema load · Line 193-193
  
  The schema.load() call could raise a ValidationError if the saved query_context doesn't match the expected schema, potentially causing the request to fail. Adding error handling here would make the code more robust by falling back to form_data construction.

Review Details

Files reviewed - 3 · Commit Range: d83b539..792884e
- superset/mcp_service/chart/schemas.py
- superset/mcp_service/chart/tool/get_chart_data.py
- tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.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 [email protected].

Documentation & Help

AI Code Review powered by

Addresses Vitor's feedback: instead of defaulting to 100 rows, now uses the chart's configured row_limit from form_data. Users can still override with request.limit, and MAX_ROW_LIMIT (10000) remains as a safety cap. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

bito-code-review · 2026-01-07T17:38:25Z

Code Review Agent Run #277619

Actionable Suggestions - 0

Additional Suggestions - 1

superset/mcp_service/chart/tool/get_chart_data.py - 1
- Potential Runtime Error · Line 166-166
  
  The change improves row_limit handling by respecting the chart's configured limit (defaulting to 1000 instead of 100), but introduces a potential TypeError if form_data contains `"row_limit": null`—since `form_data.get("row_limit", 1000)` returns null when the key exists with a null value, leading to `min(request.limit or null, MAX_ROW_LIMIT)` failing. This could cause runtime exceptions for charts with malformed params.
  Code suggestion
  @@ -164,4 +164,4 @@ # Use request.limit if specified, otherwise use chart's configured # row_limit, with MAX_ROW_LIMIT as safety cap - chart_row_limit = form_data.get("row_limit", 1000) + chart_row_limit = form_data.get("row_limit") or 1000 row_limit = min(request.limit or chart_row_limit, MAX_ROW_LIMIT)

Review Details

Files reviewed - 3 · Commit Range: 792884e..bebda26
- superset/mcp_service/chart/schemas.py
- superset/mcp_service/chart/tool/get_chart_data.py
- tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.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 [email protected].

Documentation & Help

AI Code Review powered by

Per review feedback, charts already have enforced row limits from global config (ROW_LIMIT, SQL_MAX_ROW), so the MCP-specific MAX_ROW_LIMIT is unnecessary. This change: - Removes MAX_ROW_LIMIT constant from get_chart_data.py - Updates schema description to remove "capped at 10000" text - Removes MAX_ROW_LIMIT tests that are no longer needed - Respects chart's configured row_limit or request.limit directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Uses the same pattern as viz.py for fallback row_limit: request.limit or form_data.get("row_limit") or current_app.config["ROW_LIMIT"] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

bito-code-review · 2026-01-07T19:35:57Z

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it

apache#36937) Co-authored-by: Claude Opus 4.5 <[email protected]>

pull-request-size bot added the size/M label Jan 7, 2026

dosubot bot added the api Related to the REST API label Jan 7, 2026

codeant-ai-for-open-source bot reviewed Jan 7, 2026

View reviewed changes

superset/mcp_service/chart/tool/get_chart_data.py Outdated Show resolved Hide resolved

pull-request-size bot added size/L and removed size/M labels Jan 7, 2026

github-actions bot removed the api Related to the REST API label Jan 7, 2026

Vitor-Avila reviewed Jan 7, 2026

View reviewed changes

superset/mcp_service/chart/tool/get_chart_data.py Outdated Show resolved Hide resolved

Vitor-Avila approved these changes Jan 7, 2026

View reviewed changes

aminghadersohi changed the title ~~fix(mcp): add missing @parse_request decorator to get_chart_data and add raw data option~~ fix(mcp): use chart.query_context for get_chart_data like the API does Jan 7, 2026

aminghadersohi and others added 3 commits January 7, 2026 13:33

chore(mcp): remove redundant API reference comments

eddfea6

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Vitor-Avila merged commit 64ee48f into apache:master Jan 7, 2026
65 checks passed

JCelento pushed a commit to JCelento/superset that referenced this pull request Jan 8, 2026

fix(mcp): use chart.query_context for get_chart_data like the API does (

53c6a4e

apache#36937) Co-authored-by: Claude Opus 4.5 <[email protected]>

fix(mcp): use chart.query_context for get_chart_data like the API does #36937

fix(mcp): use chart.query_context for get_chart_data like the API does #36937

Conversation

aminghadersohi commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Verified Fix with Temporal Line Chart

Tested Fallback Path (charts without saved query_context)

Additional Information

Reported Issues (by @villebro)

Root Causes

Key Fix

Uh oh!

codeant-ai-for-open-source bot commented Jan 7, 2026

Thanks for using CodeAnt! 🎉

Uh oh!

bito-code-review bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #047a2b

Uh oh!

codeant-ai-for-open-source bot commented Jan 7, 2026

Nitpicks 🔍

Uh oh!

netlify bot commented Jan 7, 2026

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

Uh oh!

codeant-ai-for-open-source bot commented Jan 7, 2026

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aminghadersohi commented Jan 7, 2026

Addressing Review Feedback

✅ Fixed: Unbounded row fetch risk

✅ Added: Unit tests for new feature

ℹ️ Re: Backward-incompatible schema change (False Positive)

Uh oh!

bito-code-review bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #87d875

Uh oh!

codeant-ai-for-open-source bot commented Jan 7, 2026

Thanks for using CodeAnt! 🎉

Uh oh!

Uh oh!

Vitor-Avila left a comment

Choose a reason for hiding this comment

Uh oh!

bito-code-review bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #9e29fa

Uh oh!

bito-code-review bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #277619

Uh oh!

Uh oh!

bito-code-review bot commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aminghadersohi commented Jan 7, 2026 •

edited

Loading

Tested Fallback Path (charts without saved `query_context`)

bito-code-review bot commented Jan 7, 2026 •

edited

Loading

codecov bot commented Jan 7, 2026 •

edited

Loading

bito-code-review bot commented Jan 7, 2026 •

edited

Loading

bito-code-review bot commented Jan 7, 2026 •

edited

Loading

bito-code-review bot commented Jan 7, 2026 •

edited

Loading