-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Improve documentation for the read_multiple_files action
#2602
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
Improve documentation for the read_multiple_files action
#2602
Conversation
b11c9e0 to
fbec84b
Compare
olaservo
left a 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.
Thanks for the PR! In order to fully address this issue, should the tool description also be updated? #2032
|
I'm assuming you are referring to enhancing this description: Are you suggesting the input parameters should be documented in the tool description field? The MCP tool specification defines three distinct documentation layers for the description fields:
Established Patterns for description fieldsLooking at established patterns for the description field, I see inconsistencies. Sometimes the tool description is high-level without parameter descriptions. And sometimes the tool description includes parameter descriptions. Filesystem Server:
Memory Server: Tool descriptions are consistently high-level:
NO parameter names or structures mentioned in tool descriptions Git Server: Tool descriptions are brief and functional:
NO parameter details in tool descriptions How LLMs Use Tool Documentation in PracticeA key consideration is how LLMs Use Tool Documentation in Practice. Looking at VS Code, Claude desktop and Claude code, I see the LLMs use the following pattern:
Both description fields are read before invoking the tool, as they serve complementary, not redundant purposes. Note this is just anecdotal evidence, I'm not sure if this pattern is widespread across all LLMs. I've also noticed the following behavior:
Problems with Including Parameters in Tool DescriptionsIMHO, it makes sense to keep the tool description high-level otherwise it becomes redundant with the input schema description, potentially leading to maintenance issues and contradictions/ambiguities. Including Parameters in Tool Descriptions can cause problems:
Benefits of SeparationThe separation of the tool description and input schema descriptions allows for:
Potential Enhancements to the MCP SpecI think there is a genuine ambiguity in the MCP spec that could benefit from clarification. For example, there could be documentation best practices. I have created modelcontextprotocol/modelcontextprotocol#1382 |
fbec84b to
b647cb3
Compare
|
Thanks @sebastien-rosset , looking at the tool description again, I think you're correct that there isn't a need to specify anything differently there. I agree that we shouldn't be repeating the information given by tool params given that the model should already see those (and tbh the name of the function already implies that a list or array of files should be passed in, so not sure why models are struggling with using it correctly in the first place). |
Description
This PR fixes the
read_multiple_filesfunction to properly handle empty arrays and improves documentation to make the parameter structure self-documenting for AI assistants.Server Details
Motivation and Context
Addresses issue #2032 where the
read_multiple_filesfunction lacked clear documentation about its parameter structure and behavior, causing AI assistants to use it incorrectly without explicit instruction. The function would accept empty arrays and return empty results without helpful feedback, making it difficult for AI assistants to understand proper usage.How Has This Been Tested?
Breaking Changes
No breaking changes for valid usage. The only change is that empty arrays (which previously returned empty results) now return a clear error message explaining that at least one file path is required.
Types of changes
Checklist
Additional context
Changes made:
.min(1, "At least one file path must be provided")to reject empty arrays with a clear error message.describe()to thepathsparameter explaining the expected format and requirementsBefore this fix:
After this fix:
The fix maintains full backward compatibility for all valid use cases while providing better guidance for incorrect usage.