Prompt: Add general and MCP tool instructions to system prompt of MCP server #53
Conversation
|
""" WalkthroughThis update introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant MainScript as __main__.py
participant Resources as importlib.resources
participant FastMCP as FastMCP
participant InstructionsFile as instructions.md
participant InstructionsGeneral as GeneralInstructions
MainScript->>Resources: Load instructions.md text
Resources-->>MainScript: Return instructions text
MainScript->>InstructionsGeneral: Retrieve general instructions text
InstructionsGeneral-->>MainScript: Return general instructions text
MainScript->>FastMCP: Initialize(name, instructions_mcp + instructions_general)
FastMCP-->>MainScript: Application ready with combined instructions
Estimated code review effort2 (~20 minutes) Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ 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
🧹 Nitpick comments (1)
cratedb_mcp/instructions.md (1)
23-23: Minor style improvement: simplify "return back" to "return".As flagged by the static analysis tool, "return back" is redundant - "return" alone is clearer and more concise.
-Your task is to translate questions into SQL queries, run them on CrateDB, and return back results. +Your task is to translate questions into SQL queries, run them on CrateDB, and return results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGES.md(1 hunks)cratedb_mcp/__main__.py(2 hunks)cratedb_mcp/instructions.md(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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#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.
CHANGES.md (3)
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#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.
pyproject.toml (3)
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#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.
cratedb_mcp/__main__.py (3)
Learnt from: amotl
PR: crate/cratedb-mcp#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: 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.
cratedb_mcp/instructions.md (2)
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#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.
🪛 LanguageTool
cratedb_mcp/instructions.md
[style] ~23-~23: Consider using just “return”.
Context: ...o SQL queries, run them on CrateDB, and return back results. - Try to generate SQL queries ...
(RETURN_BACK)
🔇 Additional comments (5)
pyproject.toml (1)
97-101: LGTM! Package data configuration correctly implemented.The setuptools configuration properly enables inclusion of Markdown files as package data, which will ensure the new
instructions.mdfile is bundled with the package distribution.CHANGES.md (1)
4-5: LGTM! Changelog entry properly documents the feature addition.The entry clearly describes the addition of instructions to the MCP server and includes appropriate contributor attribution.
cratedb_mcp/__main__.py (2)
1-1: LGTM! Modern approach to importing package resources.Using
importlib.resourcesis the recommended way to access package data files in modern Python.
15-21: LGTM! Instructions loading and FastMCP integration implemented correctly.The implementation properly loads the instructions file using the modern
importlib.resourcesAPI and correctly passes the content to the FastMCP application instance.cratedb_mcp/instructions.md (1)
1-80: Excellent comprehensive instructions for CrateDB MCP server.The instructions file provides thorough guidance covering CrateDB architecture, SQL query rules, tool usage, and writing principles. The content correctly emphasizes key differences between CrateDB and similar systems (PostgreSQL, Elasticsearch) and includes specific technical guidance like using
MAX_BYfor latest values and proper interval syntax.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_instructions.py (1)
4-14: Good test coverage for instruction loading verification.The test effectively verifies that the MCP server has been initialized with the expected instruction sections. The use of string containment checks is appropriate for ensuring key content is present.
Consider enhancing the test robustness by checking for instruction content more flexibly to handle potential text variations:
def test_instructions(): instructions_text = mcp.instructions + assert instructions_text, "Instructions should not be empty" + + # Convert to lowercase for case-insensitive matching + instructions_lower = instructions_text.lower() # MCP instructions. - assert "MCP tool instructions" in instructions_text + assert "mcp tool instructions" in instructions_lower # General instructions. - assert "Things to remember when working with CrateDB" in instructions_text - assert "Rules for writing SQL queries" in instructions_text - assert "Core writing principles" in instructions_text + assert "things to remember when working with cratedb" in instructions_lower + assert "rules for writing sql queries" in instructions_lower + assert "core writing principles" in instructions_lowerThis would make the test more resilient to minor formatting changes while still ensuring the essential content is present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGES.md(1 hunks)cratedb_mcp/__main__.py(2 hunks)cratedb_mcp/instructions.md(1 hunks)pyproject.toml(2 hunks)tests/test_instructions.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- pyproject.toml
- CHANGES.md
- cratedb_mcp/main.py
- cratedb_mcp/instructions.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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#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.
tests/test_instructions.py (2)
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#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.
🔇 Additional comments (1)
tests/test_instructions.py (1)
1-2: LGTM! Clean import structure.The import statement correctly imports the
mcpobject from the main module, following standard conventions.
amotl
changed the title
pyproject.toml
Outdated
| "cachetools<7", | ||
| "click<9", | ||
| "cratedb-about==0.0.5", | ||
| "cratedb-about @ git+https://github.com/crate/about.git@instructions", |
There was a problem hiding this comment.
This needs a release first, depending on another patch.
It's also why CI is failing, because it probes an upload to PyPI, which is rejected because of that spot. Please review both patches. If they are acknowledged, we are able to land them in lock-step.
There was a problem hiding this comment.
v0.0.6 has been released.
cratedb_mcp/instructions.md
Outdated
| 1. `get_table_metadata`: Return column schema and metadata for all tables stored in CrateDB. Use it to inquire entities you don't know about. | ||
| 2. `query_sql`: Execute SQL query on CrateDB and return results. | ||
| 3. `get_cratedb_documentation_index`: Returns the table of contents for the CrateDB documentation. If in doubt about CrateDB-specific syntax, you can obtain the documentation here. | ||
| 4. `fetch_cratedb_docs`: Once a specific link within the CrateDB documentation is identified, you can download its content here by providing the link. |
There was a problem hiding this comment.
I know it was me who wrote those, but it already felt slightly wrong to me when I wrote it 🙂. __main__.py defines the tools, and each of those has a description. In my understanding, an agent would do some sort of tool discovery and consume descriptions from there. Which means we should either look into extending those descriptions (if not sufficient) or tweak the system prompt to force the consumption of those descriptions?
There was a problem hiding this comment.
In my understanding, an agent would do some sort of tool discovery and consume descriptions from there.
That's true. I had the same thoughts, and will investigate the situation. Thanks.
There was a problem hiding this comment.
So, the thing is: It is apparently totally at the discretion of the MCP client to use the tools, after invoking the list_tools API method of the MCP server. Apparently, this is not guaranteed, and also, it is not enough: While individual tools could inform the prompt about their individual capabilities, it would not be sufficient to relay guidelines how to use multiple tools in combination, just to enumerate a single example. In this spirit, there must always be a separate set of guidelines that embrace the whole ensemble. It is also defined by the FastMCP server like this:
instructions: Description of how to interact with this server. These instructions help clients understand the server’s purpose and available functionality.
- https://gofastmcp.com/servers/server#param-instructions
- https://www.speakeasy.com/mcp/using-mcp/using-tools#tell-the-llm-which-tools-to-use-and-when
- Quality: Provide "instructions.md" #26
You can also explore a similar observation when using other LLM frameworks that can use MCP tools these days: While you can easily import available tools, more often than not, you will need to supply usage instructions separately:
- LlamaIndex: Add example using MCP cratedb-examples#1032
- https://pypi.org/project/llama-index-tools-mcp/
- https://docs.llamaindex.ai/en/stable/understanding/tracing_and_debugging/tracing_and_debugging/
- https://github.com/run-llama/llama_index/blob/main/llama-index-integrations/tools/llama-index-tools-mcp/examples/mcp.ipynb
Salvatore's post Coding with LLMs in the summer of 2025 also emphasizes the detail to be in absolute control of your context prompts, so I also think it is better to be explicit and provide excellent human-written guidelines for MCP server tool use.
Avoid any RAG that will show only part of the code / context to the LLM. This destroys LLMs performance. You must be in control of what the LLM can see when providing a reply.
There was a problem hiding this comment.
In my understanding, an agent would do some sort of tool discovery and consume descriptions from there. Which means we should either look into extending those descriptions (if not sufficient) or tweak the system prompt to force the consumption of those descriptions?
Yeah, tool discovery is present, and I agree what you are describing would resemble a perfect world situation, but still people are providing human-written guidelines, and recommend to do it, because maybe LLMs will just not be as smart as we would like them to be.
-
Tell the LLM which tools to use and when
Rather than hoping the LLM will choose the right tools, be explicit in your instructions: [...]
-
Flux MCP Server Prompting Guide
Providing instructions is crucial for guiding the behavior of your AI assistant when interacting with the Flux MCP Server. We've created a set of instructions (1400 tokens) that you can use as a starting point.
There was a problem hiding this comment.
Coming from tell the LLM which tools to use and when, I think we should spice up our specific instructions, for example by caring about tool sequencing:
When tasks require multiple steps, outline the expected tool sequence. For example, “First use
web_searchto find recent articles, thenfetch_urlto get full content, thensummarize_textto create a brief overview.”
While we only provide two pairs of tools, they can still be sequenced properly, so the LLM does not need to reason without supervision or missing clues, while providing them would be so easy.
There was a problem hiding this comment.
Thanks for the research, it sounds plausible that we need to tell the LLM how to best combine the tools.
I think we should spice up our specific instructions
Yes, what I wanted to express is that I couldn't really validate if the instructions I came up with initially made a difference or not. It is probably one of the major difficulties to tell what part of a prompt actually makes a difference in how the LLM processes a question, and what doesn't. Since it depends on the model, one probably cannot simply tell, and hence one needs to be extra verbose.
There was a problem hiding this comment.
Yes, it looks like common practice. @WalBeh had good experiences leveling up LLM understanding with those techniques when using gpt-4o or 4.1 models, and also sonnet.
There was a problem hiding this comment.
It sounds plausible that we need to tell the LLM how to best combine the tools.
0d1b65c adds two more rules that put the tools in context to each other, i.e. improving details about "tool sequencing", aka. how to combine them well.
Please add any feedback about them also in retrospective when applicable, or just send patches to fix forward ;]. Thanks!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cratedb_mcp/instructions.md (2)
8-12: Align tool descriptions for tense & clarityThe list mixes imperative (“Execute”) and present-tense (“Returns”). Unifying the voice improves readability and avoids cognitive friction when these sentences are injected into the system prompt.
-1. `get_table_metadata`: Return column schema and metadata for all tables stored in CrateDB. Use it to inquire entities you don't know about. -2. `query_sql`: Execute SQL query on CrateDB and return results. -3. `get_cratedb_documentation_index`: Returns the table of contents for the CrateDB documentation. If in doubt about CrateDB-specific syntax, you can obtain the documentation here. -4. `fetch_cratedb_docs`: Once a specific link within the CrateDB documentation is identified, you can download its content here by providing the link. +1. `get_table_metadata`: Returns column schema and metadata for all tables stored in CrateDB. Use it when you need to discover entities you are unfamiliar with. +2. `query_sql`: Executes an SQL query on CrateDB and returns the results. +3. `get_cratedb_documentation_index`: Returns the table of contents of the CrateDB documentation. Use it whenever you need to verify CrateDB-specific syntax. +4. `fetch_cratedb_docs`: Given a link returned by `get_cratedb_documentation_index`, fetches the full content of that documentation page.
15-24: Add best-practice hints for SQL & doc retrieval flowsTwo micro-tweaks tighten the guidance:
- Encourage parameterised, column-specific queries with a
LIMITto avoid overwhelming result sets.- Clarify that
fetch_cratedb_docsshould be used after the index lookup and that the agent should interpret results before answering.- column names and types, then use `query_sql` to query the database using SQL, based on - user's inquiries and questions. + column names and types. Next, use `query_sql` to execute a parameterised SQL query that + returns only the columns you need (avoid `SELECT *`) and, where appropriate, add a + `LIMIT` clause to keep result sets concise. - about CrateDB, then use `fetch_cratedb_docs` to fetch individual documentation pages that - can be used to answer questions about CrateDB precisely. + about CrateDB. Then, use `fetch_cratedb_docs` to retrieve individual pages that + can be quoted verbatim when answering questions about CrateDB. -Try to reason and give an interpretation of the result. +After fetching data, reason about the output and provide a concise interpretation before +formulating the final answer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cratedb_mcp/instructions.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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#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/instructions.md (4)
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: #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: #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: #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.
⏰ 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
... because other frameworks can import MCP tools as well. So, it is better to use a section name that works generally across all tools?
|
@coderabbitai: Thanks, we've used your suggestions to improve the writing style per 43e449f. |
|
Tip For best results, initiate chat on the files or code changes. @amotl You're very welcome! I'm glad the writing style suggestions were helpful in improving the instructions. It's great to see how responsive you are to feedback - this collaborative approach really helps enhance the quality of the codebase. The updated instructions should now provide clearer guidance for users working with the MCP tools. Thanks for taking the time to incorporate the suggestions! 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
About
Instructions about how to use provided MCP tools have been missing dearly. @hammerhead and @WalBeh contributed a few. Thanks!
Details
I've stacked different kinds of guidelines on top of each other. Future iterations might remix and bring them into shape further. Please add any of your thoughts for improvements to the review comments.
What's inside
Preview
uv tool install --upgrade 'cratedb-mcp @ git+https://github.com/crate/cratedb-mcp.git@instructions'References
Backlog