Skip to content

fix(mcp): remove @parse_request decorator for cleaner tool schemas#38918

Merged
aminghadersohi merged 3 commits intoapache:masterfrom
aminghadersohi:amin/remove-parse-request-decorator
Mar 29, 2026
Merged

fix(mcp): remove @parse_request decorator for cleaner tool schemas#38918
aminghadersohi merged 3 commits intoapache:masterfrom
aminghadersohi:amin/remove-parse-request-decorator

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

@aminghadersohi aminghadersohi commented Mar 27, 2026

User description

SUMMARY

The @parse_request decorator was originally added as a workaround for a Claude Code double-serialization bug (fastmcp#5504) that wrapped tool parameters in a confusing anyOf [string, object] schema under a request key. This caused two problems reported by users:

  1. LLMs frequently forgot the request wrapper, sending flat params like {"chart_id": 62} instead of {"request": {"identifier": 62}}, causing validation errors before self-correcting
  2. LLMs used wrong parameter names (e.g. title vs dashboard_title) because the anyOf schema was unclear to models

With FastMCP 3.1, Tool.from_function handles Pydantic BaseModel parameters natively, generating clean JSON schemas. This makes @parse_request unnecessary.

Changes:

  • Remove @parse_request decorator and its import from all 19 tool files
  • Add Context injection in mcp_auth_hook (previously handled by @parse_request) via _needs_ctx detection and _inject_ctx helper
  • Add default value for get_instance_info's empty request model (previously handled by @parse_request)
  • Remove from __future__ import annotations from execute_sql and save_sql_query (string annotations prevent FastMCP from recognizing the Context parameter, causing silent tool registration failures)

Schema improvement (before → after):

# Before: confusing anyOf wrapping
"request": {"anyOf": [{"type": "string"}, {"$ref": "#/$defs/ListChartsRequest"}]}

# After: clean object type
"request": {"properties": {"page": ..., "page_size": ..., ...}, "type": "object"}

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend-only change affecting MCP tool JSON schemas.

TESTING INSTRUCTIONS

  1. Start the MCP service (python -m superset.mcp_service.server)
  2. Connect with an MCP client (Claude Code, Claude Desktop, etc.)
  3. Verify tools work correctly:
    • get_instance_info (no args needed)
    • list_charts with {"request": {"page": 1, "page_size": 5}}
    • execute_sql with {"request": {"database_id": 1, "sql": "SELECT 1"}}
    • save_sql_query with {"request": {"database_id": 1, "label": "Test", "sql": "SELECT 1"}}
  4. Verify tool schemas no longer contain anyOf [string, object] patterns
  5. Verify all 20 tools register without errors in startup logs

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

CodeAnt-AI Description

Make MCP tool inputs simpler and more reliable

What Changed

  • MCP tools now accept their request fields directly without the extra request wrapper, which reduces validation errors from LLM clients
  • Context is still passed to tools behind the scenes, so discovery and action tools continue to work without changing user-facing behavior
  • The instance info tool now works even when no request body is sent, instead of requiring an empty object
  • SQL execution and SQL save tools no longer depend on string-based annotations, which restores normal tool registration

Impact

✅ Fewer tool input validation errors
✅ Cleaner MCP tool schemas for LLM clients
✅ Fewer missing-tool failures in SQL Lab actions

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

The @parse_request decorator was a workaround for a Claude Code
double-serialization bug (GitHub issue apache#5504) that wrapped tool
parameters in a confusing anyOf [string, object] schema under a
"request" key. This caused two problems reported by users:

1. LLMs frequently forgot the "request" wrapper, sending flat params
   like {"chart_id": 62} instead of {"request": {"identifier": 62}}
2. LLMs used wrong parameter names (e.g. "title" vs "dashboard_title")
   because the schema was unclear

With FastMCP 3.1, Tool.from_function handles Pydantic BaseModel
parameters natively, making @parse_request unnecessary.

Changes:
- Remove @parse_request decorator and import from all 19 tool files
- Add Context injection in mcp_auth_hook (previously done by
  @parse_request) via _needs_ctx detection and _inject_ctx helper
- Add default value for get_instance_info's empty request model
- Remove from __future__ import annotations from execute_sql and
  save_sql_query (string annotations prevent FastMCP from recognizing
  the Context parameter, causing tool registration failures)
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Mar 27, 2026

Code Review Agent Run #d3c6b4

Actionable Suggestions - 0
Additional Suggestions - 5
  • superset/mcp_service/dataset/tool/list_datasets.py - 2
    • Missing import for request parsing · Line 43-43
      Removing this import disables the @parse_request decorator, which is needed to handle JSON string inputs from MCP clients like Claude Code. Without it, the tool may fail when receiving string-serialized requests instead of dicts.
      Code suggestion
       @@ -43,1 +43,1 @@
      - 
      +from superset.mcp_service.utils.schema_utils import parse_request
    • Removed request parsing decorator · Line 73-73
      The @parse_request decorator is recommended in CLAUDE.md for handling double-serialized requests from Claude Code. Removing it may cause failures when the client sends JSON strings.
  • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py - 1
    • Missing preferred decorator · Line 317-317
      The CLAUDE.md guideline prefers using the @parse_request decorator for MCP tool functions to handle request parsing automatically. Removing it creates inconsistency with other tools and may affect input handling for string/object formats.
  • superset/mcp_service/chart/tool/get_chart_preview.py - 1
    • Input parsing compatibility risk · Line 2189-2189
      Removing the @parse_request decorator may break compatibility with MCP clients that send request parameters as JSON strings rather than parsed objects. The CLAUDE.md guide states that tools should accept both formats using this decorator. If the MCP framework now handles Pydantic parsing automatically, consider updating the guide; otherwise, this could cause runtime errors for string inputs.
  • superset/mcp_service/dashboard/tool/list_dashboards.py - 1
    • Removed parse_request decorator against guidelines · Line 75-75
      Removing the @parse_request decorator violates the preferred practice in CLAUDE.md, which recommends it for automatic handling of string inputs from MCP clients. This may reduce compatibility with certain clients.
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • superset/mcp_service/system/tool/get_instance_info.py - 1
  • superset/mcp_service/explore/tool/generate_explore_link.py - 1
  • superset/mcp_service/chart/tool/update_chart_preview.py - 1
    • Runtime Error: Missing Request Parsing · Line 57-57
  • superset/mcp_service/dataset/tool/get_dataset_info.py - 1
  • superset/mcp_service/chart/tool/get_chart_data.py - 1
    • Removed preferred @parse_request decorator · Line 86-86
Review Details
  • Files reviewed - 20 · Commit Range: 505cf95..505cf95
    • superset/mcp_service/auth.py
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/chart/tool/get_chart_data.py
    • superset/mcp_service/chart/tool/get_chart_info.py
    • superset/mcp_service/chart/tool/get_chart_preview.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/chart/tool/update_chart.py
    • superset/mcp_service/chart/tool/update_chart_preview.py
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • superset/mcp_service/dashboard/tool/get_dashboard_info.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/tool/get_dataset_info.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/explore/tool/generate_explore_link.py
    • superset/mcp_service/sql_lab/tool/execute_sql.py
    • superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
    • superset/mcp_service/sql_lab/tool/save_sql_query.py
    • superset/mcp_service/system/tool/get_instance_info.py
    • superset/mcp_service/system/tool/get_schema.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

@dosubot dosubot bot added the change:backend Requires changing the backend label Mar 27, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:M This PR changes 30-99 lines, ignoring generated files label Mar 27, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

❌ Patch coverage is 46.66667% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.51%. Comparing base (fc705d9) to head (236423d).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/auth.py 42.85% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #38918      +/-   ##
==========================================
- Coverage   65.81%   64.51%   -1.31%     
==========================================
  Files        1823     2536     +713     
  Lines       73196   130755   +57559     
  Branches    23460    30338    +6878     
==========================================
+ Hits        48175    84353   +36178     
- Misses      25021    44939   +19918     
- Partials        0     1463    +1463     
Flag Coverage Δ
hive 40.19% <46.66%> (?)
mysql 61.14% <46.66%> (?)
postgres 61.22% <46.66%> (?)
presto 40.21% <46.66%> (?)
python 62.82% <46.66%> (?)
sqlite 60.85% <46.66%> (?)
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.

Ruff B008 disallows function calls in argument defaults. Move the
GetSupersetInstanceInfoRequest() default to a module-level singleton.
- Pass mock_ctx explicitly in save_sql_query tests since ctx is
  now a regular parameter (not injected by @parse_request)
- Remove test_get_schema_with_json_string_request test that was
  testing @parse_request JSON string handling (no longer exists)
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 27, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Mar 27, 2026

Code Review Agent Run #881fe2

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 505cf95..236423d
    • superset/mcp_service/system/tool/get_instance_info.py
    • tests/unit_tests/mcp_service/sql_lab/tool/test_save_sql_query.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.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

@aminghadersohi aminghadersohi merged commit d1903af into apache:master Mar 29, 2026
67 checks passed
michael-s-molina pushed a commit that referenced this pull request Mar 30, 2026
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/L size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants