MCP: Start using MCP tool descriptions from Python docstrings #65
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update replaces hardcoded tool descriptions with Python docstrings for several CrateDB MCP tools and swaps the functions assigned to two "Text-to-SQL" tools. It also improves docstrings for clarity, revises user instructions, updates the changelog, and adjusts related tests to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MCP
participant ToolFunction
User->>MCP: Request with tool usage
MCP->>ToolFunction: Use function docstring as tool description
MCP->>ToolFunction: Call assigned function (e.g., query_sql, get_table_metadata)
ToolFunction-->>MCP: Return result
MCP-->>User: Respond with result and updated tool description
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGES.md(1 hunks)cratedb_mcp/core.py(3 hunks)cratedb_mcp/tool.py(4 hunks)tests/test_knowledge.py(1 hunks)tests/test_mcp.py(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the `backlog.md` document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.
Learnt from: amotl
PR: crate/cratedb-mcp#60
File: cratedb_mcp/tool.py:43-43
Timestamp: 2025-07-22T12:46:59.375Z
Learning: In the cratedb-mcp repository, the DocumentationIndex class has a method named `to_dict()` that actually returns a list, not a dictionary. This misleading method name can cause confusion about return types in functions that call it, such as `get_cratedb_documentation_index()`. A ticket was created to fix this semantic naming issue: https://github.com/crate/about/issues/55
Learnt from: amotl
PR: crate/cratedb-mcp#54
File: tests/test_cli.py:0-0
Timestamp: 2025-07-20T17:17:10.660Z
Learning: In the cratedb-mcp project, tests should avoid asserting against `mcp.settings` internal state as these assertions will break with later versions of FastMCP. Tests should focus on verifying that functions are called with correct parameters rather than checking internal implementation details.
tests/test_mcp.py (1)
Learnt from: amotl
PR: #54
File: tests/test_cli.py:0-0
Timestamp: 2025-07-20T17:17:10.660Z
Learning: In the cratedb-mcp project, tests should avoid asserting against mcp.settings internal state as these assertions will break with later versions of FastMCP. Tests should focus on verifying that functions are called with correct parameters rather than checking internal implementation details.
CHANGES.md (5)
Learnt from: amotl
PR: #17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the backlog.md document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.
Learnt from: amotl
PR: #24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
Learnt from: amotl
PR: #54
File: tests/test_cli.py:0-0
Timestamp: 2025-07-20T17:17:10.660Z
Learning: In the cratedb-mcp project, tests should avoid asserting against mcp.settings internal state as these assertions will break with later versions of FastMCP. Tests should focus on verifying that functions are called with correct parameters rather than checking internal implementation details.
Learnt from: amotl
PR: #60
File: cratedb_mcp/tool.py:43-43
Timestamp: 2025-07-22T12:46:59.375Z
Learning: In the cratedb-mcp repository, the DocumentationIndex class has a method named to_dict() that actually returns a list, not a dictionary. This misleading method name can cause confusion about return types in functions that call it, such as get_cratedb_documentation_index(). A ticket was created to fix this semantic naming issue: crate/about#55
cratedb_mcp/core.py (2)
Learnt from: amotl
PR: #60
File: cratedb_mcp/tool.py:43-43
Timestamp: 2025-07-22T12:46:59.375Z
Learning: In the cratedb-mcp repository, the DocumentationIndex class has a method named to_dict() that actually returns a list, not a dictionary. This misleading method name can cause confusion about return types in functions that call it, such as get_cratedb_documentation_index(). A ticket was created to fix this semantic naming issue: crate/about#55
Learnt from: amotl
PR: #17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
cratedb_mcp/tool.py (4)
Learnt from: amotl
PR: #17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: #60
File: cratedb_mcp/tool.py:43-43
Timestamp: 2025-07-22T12:46:59.375Z
Learning: In the cratedb-mcp repository, the DocumentationIndex class has a method named to_dict() that actually returns a list, not a dictionary. This misleading method name can cause confusion about return types in functions that call it, such as get_cratedb_documentation_index(). A ticket was created to fix this semantic naming issue: crate/about#55
Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the backlog.md document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.
Learnt from: amotl
PR: #54
File: tests/test_cli.py:0-0
Timestamp: 2025-07-20T17:17:10.660Z
Learning: In the cratedb-mcp project, tests should avoid asserting against mcp.settings internal state as these assertions will break with later versions of FastMCP. Tests should focus on verifying that functions are called with correct parameters rather than checking internal implementation details.
🧬 Code Graph Analysis (4)
tests/test_mcp.py (1)
cratedb_mcp/tool.py (1)
get_table_metadata(31-39)
cratedb_mcp/core.py (1)
cratedb_mcp/tool.py (5)
query_sql(20-28)get_table_metadata(31-39)get_cratedb_documentation_index(50-55)fetch_cratedb_docs(58-66)get_cluster_health(72-74)
tests/test_knowledge.py (1)
cratedb_mcp/knowledge.py (1)
Queries(11-91)
cratedb_mcp/tool.py (2)
cratedb_mcp/util/sql.py (1)
sql_is_permitted(13-31)cratedb_mcp/knowledge.py (2)
DocumentationIndex(94-157)items(136-140)
🪛 GitHub Actions: Tests
tests/test_mcp.py
[error] 61-61: Test failure in test_get_table_metadata: AssertionError - expected substring 'information_schema.tables' not found in the returned metadata string.
⏰ 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). (2)
- GitHub Check: oci
- GitHub Check: oci
🔇 Additional comments (13)
cratedb_mcp/tool.py (6)
12-12: Well-written docstring enhancement.The docstring clearly and concisely describes the function's purpose and aligns perfectly with the implementation.
21-25: Excellent docstring with practical guidance.The enhanced docstring not only describes the function's purpose but also provides valuable guidance about SQL best practices, which will be particularly helpful for LLM-generated queries.
33-33: Clear scope enhancement in docstring.The updated docstring explicitly clarifies that the function returns metadata for "all tables stored in CrateDB," providing better context about the scope of the returned data.
51-54: Comprehensive docstring with usage guidance.The enhanced multi-line docstring effectively describes both the function's purpose and provides clear guidance on when to use it, which will help LLMs make appropriate tool selection decisions.
59-63: Excellent contextual docstring.The enhanced docstring effectively explains the function's relationship to
get_cratedb_documentation_indexand provides important usage information about quoting content verbatim, creating a clear workflow for LLMs.
73-73: Good enhancement with implementation details.The updated docstring effectively describes both the function's purpose and provides useful implementation details about querying
sys.healthand the ordering of results.CHANGES.md (1)
4-4: Accurate changelog entry.The changelog entry clearly and concisely documents the main change in this PR - using Python docstrings as MCP tool descriptions.
cratedb_mcp/core.py (5)
44-45: Consistent function and docstring update.The change correctly pairs the
query_sqlfunction with its corresponding docstring, maintaining consistency between the tool implementation and description.
51-52: Consistent completion of function swap.The change correctly pairs the
get_table_metadatafunction with its corresponding docstring, completing the logical swap between the two Text-to-SQL tools.
63-63: Correct docstring implementation.The change appropriately uses the function's enhanced docstring as the tool description, aligning with the PR's objective of using Python docstrings for tool descriptions.
70-70: Correct docstring implementation.The change appropriately uses the function's enhanced docstring, which provides excellent workflow context for the documentation fetching tool.
81-81: Correct docstring implementation.The change appropriately uses the function's enhanced docstring as the tool description, completing the systematic replacement of hardcoded descriptions with Python docstrings.
tests/test_knowledge.py (1)
16-17: Improved test precision.The changes make the test more precise by checking for
"sys.health"in the appropriateQueries.HEALTHconstant and simplifying the"partitions_health"check, resulting in more accurate and focused test assertions.
amotl
force-pushed
the
refactor-tool-descriptions
branch
from
910cd71 to
872f269
Compare
amotl
force-pushed
the
refactor-tool-descriptions
branch
from
f34b392 to
9142a8e
Compare
Pull MCP tool's
desctriptionfrom Python docstrings. Improveinstructions.md.