-
Notifications
You must be signed in to change notification settings - Fork 56
feat: reduce MCP tool token usage by removing redundant docstrings #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Conversation
Remove Args: sections from tool docstrings since FastMCP already extracts parameter info from type hints into JSON schemas. The docstring Args sections were duplicating this information. Estimated reduction: ~15-20k tokens (from ~42k to ~22-27k)
📝 WalkthroughWalkthroughMultiple tool modules in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plane_mcp/tools/work_item_properties.py (2)
62-73: Flag: Functional change contradicts PR objectives.The PR objectives state "No functional changes; tools are expected to work identically," but lines 62-73 introduce new settings processing logic that converts generic
dictsettings into type-specificTextAttributeSettingsorDateAttributeSettingsobjects based on theproperty_type.While this may be a useful enhancement, it represents a behavioral change that should either be:
- Documented in the PR description as an intentional improvement, or
- Moved to a separate PR focused on functional enhancements
131-142: Flag: Functional change contradicts PR objectives.Similar to the
create_work_item_propertyfunction, lines 131-142 introduce new settings processing logic that wasn't present before. This conditional transformation ofsettingsbased onproperty_typeis a functional change that contradicts the PR's stated objective of having "No functional changes."Consider moving this enhancement to a separate PR or updating the PR description to document this behavioral change.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plane_mcp/tools/cycles.pyplane_mcp/tools/initiatives.pyplane_mcp/tools/intake.pyplane_mcp/tools/modules.pyplane_mcp/tools/projects.pyplane_mcp/tools/users.pyplane_mcp/tools/work_item_properties.pyplane_mcp/tools/work_items.py
🧰 Additional context used
🧬 Code graph analysis (4)
plane_mcp/tools/modules.py (1)
plane_mcp/client.py (1)
get_plane_client_context(21-73)
plane_mcp/tools/cycles.py (1)
plane_mcp/client.py (1)
get_plane_client_context(21-73)
plane_mcp/tools/intake.py (1)
plane_mcp/client.py (1)
get_plane_client_context(21-73)
plane_mcp/tools/projects.py (1)
plane_mcp/client.py (1)
get_plane_client_context(21-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (13)
plane_mcp/tools/users.py (1)
13-16: LGTM! Docstring simplification aligns with PR objectives.The simplified docstring is clear and concise. FastMCP's automatic schema generation from type hints ensures parameter documentation remains available.
plane_mcp/tools/intake.py (5)
21-35: LGTM! Clear and concise docstring.The simplified docstring effectively describes the function's purpose. Parameter documentation is handled by FastMCP's schema generation from type hints.
38-49: LGTM! Documentation streamlined appropriately.The one-line docstring is sufficient, and parameter details remain accessible through the generated JSON schema.
52-69: LGTM! Docstring clarifies work item ID usage.The simplified docstring helpfully notes that the
issuefield should be used, not the intake work item ID, which is valuable guidance for users.
72-87: LGTM! Consistent simplification.The docstring maintains the important clarification about using the work item ID from the
issuefield.
90-95: LGTM! Delete operation clearly documented.The simplified docstring includes the necessary clarification about which ID to use.
plane_mcp/tools/modules.py (1)
24-33: LGTM! Consistent docstring simplification across module tools.All module-related tool docstrings have been appropriately simplified. The one-line descriptions are clear, and parameter documentation remains available through FastMCP's type hint extraction.
plane_mcp/tools/initiatives.py (1)
21-29: LGTM! Initiative tools documented concisely.The docstring simplifications are consistent with the PR objectives. All initiative operations have clear, concise descriptions that align with the pattern used across other tool modules.
plane_mcp/tools/cycles.py (1)
24-33: LGTM! Comprehensive docstring simplification for cycle tools.All 12 cycle-related tool functions have been appropriately simplified with clear, concise one-line docstrings. The changes are consistent with the PR's token reduction objectives while maintaining clarity.
plane_mcp/tools/projects.py (2)
50-218: Docstring simplifications look good.The simplified one-line docstrings for all project tool functions maintain clarity while reducing token usage, which aligns perfectly with the PR objectives. Since FastMCP extracts parameter information from type hints, the verbose
Args:sections were indeed redundant.
24-30: No action needed—this is an MCP tool with no internal Python callers to update.The
list_projectsfunction is an MCP tool decorated with@mcp.tool()and exposed through the FastMCP framework, not a regular function with internal callers. It's consumed by external clients through the MCP protocol interface rather than direct Python calls. Removingworkspace_slugfrom the signature in favor of obtaining it from context (line 32) is the correct architectural approach for MCP tools and does not require updating internal callers.plane_mcp/tools/work_items.py (1)
22-241: LGTM! Docstring simplifications are consistent and clear.All work item tool functions have been updated with concise one-line docstrings that maintain clarity. The addition of the example format "(e.g., MP-123)" in
retrieve_work_item_by_identifieris a nice touch that improves usability. No functional changes were introduced.plane_mcp/tools/work_item_properties.py (1)
27-39: Docstring simplifications look good.The simplified one-line docstrings for
list_work_item_properties,retrieve_work_item_property, anddelete_work_item_propertymaintain clarity while reducing token usage, consistent with the PR objectives.Also applies to: 96-108, 168-180
|
Thanks for the review @coderabbitai! Regarding the flagged "functional changes" in These are not new changes - the settings processing logic was already present in the git diff canary -- plane_mcp/tools/work_item_properties.pyShows only docstring removals - the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 6004 @CaliLuke, you're absolutely right, and I apologize for the error in my analysis! The git diff confirms that only docstring changes were made to The PR accurately contains only documentation changes - removing verbose Thank you for the clarification! ✨ |
| external_source: str | None = None, | ||
| order_by: str | None = None, | ||
| ) -> WorkItemSearch: | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not remove this docstring. Fields in search are little complicated and this docstring helps with that.
Please verify other tools as well once before removing all the docstrings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an eval for the search mcp? I've been using my version of the mcp with Claude since I submitted the PR and I have observed no confusion or loss of performance. Can we perhaps leave a shorter set of instructions?
Summary
Args:sections from tool docstringsImpact
Note
Cursor Bugbot is generating a summary for commit 08c3873. Configure here.
Summary by CodeRabbit
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.