chore(mcp): fix malformed tool and prompt docstrings

[aminghadersohi]

SUMMARY

FastMCP parses every registered @tool / @prompt docstring with griffelib
to derive the description and the per-argument descriptions it publishes to MCP
clients. Four superset/mcp_service/ docstrings are malformed for griffe's
Google-style parser, so parsing them logs eight WARNINGs from
griffe/_internal/docstrings/utils.py:

Failed to get 'name: description' pair from 'None - This tool does not accept any parameters'
No type or annotation for returned value 'HealthCheckResponse'
No type or annotation for parameter 'model_type'
No type or annotation for returned value 1
No type or annotation for parameter 'user_type'
No type or annotation for parameter 'focus_area'
No type or annotation for parameter 'chart_type'
No type or annotation for parameter 'business_goal'

This is per-process-start, not per-request, so every process that imports the
MCP service pays it — including workers that never serve MCP traffic. FastMCP
currently pins the griffe logger to ERROR at import, so whether the lines
reach a given deployment's logs depends on that deployment's logging config;
the malformed docstrings are the defect either way, and they are repaired here
rather than papered over with a log level.

Two of them are not merely noisy:

• health_check takes no parameters at all, so its Parameters: section
  only ever held the prose None - This tool does not accept any parameters,
  which is not a name: description pair. The sentence directly above it
  ("This tool takes NO parameters") already says this, so the section is
  dropped.
• get_schema's Args: documented model_type, which is not a parameter
  of the function — the signature is (request: GetSchemaRequest, ctx: Context).
  FastMCP only injects a docstring description when the name matches a real
  parameter, so that description was silently discarded instead of reaching the
  published input schema. It now documents request, which keeps the original
  sentence and additionally surfaces the accepted model_type values to
  clients.

Griffe parses these docstrings standalone, with no parent object to fall back
to for type information, so the remaining sections spell the type out inline
(name (type): description, (type): description) — valid Google style that
parses cleanly with or without a parent.

To keep this from regressing, a unit test walks every registered tool and
prompt, asserts that parsing its docstring emits no griffe warning, and asserts
that every documented argument is a real parameter of the function.

No change to published schemas other than the intended improvement. Dumping
the MCP tool/prompt schemas before and after shows the prompt argument
descriptions byte-identical, and the only difference is get_schema's request
property description, which grows from "Request schema for unified get_schema tool." to that sentence plus the accepted model_type values.

TESTING INSTRUCTIONS

pytest tests/unit_tests/mcp_service/test_mcp_docstrings.py

On master the guard fails with the eight warnings above; with this change it
passes. The rest of tests/unit_tests/mcp_service is unaffected.

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
• Introduces new feature or API
• Removes existing feature or API

Screenshots

	Before	After
MCP service startup. The same slice of the tool-registration sequence in both runs — same eight [INFO] lines, boxed in red/green. On master griffe logs 16 WARNING lines while FastMCP parses the tool and prompt docstrings (12 of them fall in this slice; the other 4 come from create_chart_guided earlier in the same log). On this branch it logs none.
get_schema's published request description, cropped to that field, as an MCP client receives it from tools/list. The accepted model_type values were documented against a parameter that does not exist, so FastMCP discarded them; they now reach the schema (green).

Captured at 6847d2ea. Both runs start the MCP service the way superset mcp run does and list its tools and prompts, in one container with FastMCP 3.4.7 and griffelib 2.0.2. FastMCP pins the griffe logger to ERROR when it imports its docstring parser; it is restored to its default WARNING here so the lines are visible, which is what a deployment whose logging config reloads logger levels does. Nothing else is filtered. Before = this PR's merge base 33292f24, after = the PR head; the two runs differ only in the four changed docstrings. The remaining three docstrings change no published schema byte, so they have no before/after to show.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.91%. Comparing base (33292f2) to head (6847d2e).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #44572   +/-   ##
=======================================
  Coverage   81.90%   81.91%           
=======================================
  Files        2986     2986           
  Lines      183245   183245           
  Branches    42370    42370           
=======================================
+ Hits       150090   150104   +14     
+ Misses      30418    30408   -10     
+ Partials     2737     2733    -4     

Flag	Coverage Δ
hive	36.54% <ø> (ø)
mysql	55.54% <ø> (ø)
postgres	55.54% <ø> (?)
presto	38.45% <ø> (ø)
python	85.94% <ø> (+0.01%)	⬆️
sqlite	55.27% <ø> (ø)
unit	78.91% <ø> (ø)

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.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	6847d2e
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6abee4e84725d900089a819b
😎 Deploy Preview	https://deploy-preview-44572--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

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

[bito-code-review]

Code Review Agent Run #370855

Actionable Suggestions - 0

Additional Suggestions - 3

• superset/mcp_service/chart/prompts/create_chart_guided.py - 1

  • Docstring style inconsistency · Line 33-34
    These lines switch the docstring to Google style (`Args:` + `(str)` annotations), while the rest of the repo uses Sphinx `:param:` style — `grep -rn ':param ' superset/mcp_service` finds 0 occurrences vs 68 `Args:` blocks, but the Superset codebase outside `mcp_service` uses `:param:` throughout (e.g. `superset/connectors/sqla/models.py`). Mixed styles in one codebase break `sphinx.ext.autodoc` rendering and set a divergent precedent for the next docstring edit in this file.
• tests/unit_tests/mcp_service/test_mcp_docstrings.py - 2

  • Dict key overwrites duplicate-label warnings · Line 102-102
    `_registered_functions()` deliberately returns a list because `list_tools()` can yield multiple versions of one tool (module docstring, lines 72-74), but keying `offenders` by label overwrites the earlier version's warnings. The `if messages:` guard prevents false passes, yet duplicate-label warnings are dropped from the report. Prefer `offenders.setdefault(label, []).extend(messages)`.
  • Dict key overwrites duplicate-label mismatches · Line 122-122
    Same pattern as `offenders` in `test_mcp_docstrings_parse_without_griffe_warnings`: `mismatched[label] = unknown` overwrites entries for earlier versions of a same-named tool, which `_registered_functions()` explicitly preserves (docstring lines 72-74). The `if unknown:` guard keeps the verdict correct, but earlier versions' mismatches vanish from the report. Use `mismatched.setdefault(label, []).extend(unknown)`.

Review Details

• Files reviewed - 5 · Commit Range: 3dcb205..f30b531

  • superset/mcp_service/chart/prompts/create_chart_guided.py
  • superset/mcp_service/system/prompts/quickstart.py
  • superset/mcp_service/system/tool/get_schema.py
  • superset/mcp_service/system/tool/health_check.py
  • tests/unit_tests/mcp_service/test_mcp_docstrings.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

Checked bito's style-consistency nit against the actual local convention, mcp_service is Google-style Args: throughout (68 blocks, 0 :param:), so this matches the module it's in. The get_schema fix (documenting request instead of the nonexistent model_type param) is the real substance here and it's correct. Nice touch with the before/after screenshots and the regression test. Approving.