Refactor MCP tools for token reduction#13
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll acknowledge your comments with a 👀 emoji and then get to work. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! I will automatically address your feedback. For any comments you don't want me to act on, just include (aside). For security, I will only act on instructions from the user who triggered this task for this pull request. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a large but consistent refactoring of all MCP tools to reduce token usage. The changes include shortening tool names to camelCase, shortening parameter descriptions, adding a title field, and replacing long tool descriptions with a concise summary and an example.
Overall, the changes are well-executed and achieve the goal of token reduction. However, in some cases, the descriptions have been shortened too aggressively, removing critical information that is important for the user to use the tools correctly. I've left specific comments on places where I believe some of the removed information should be restored to maintain a good balance between conciseness and clarity. For example, the distinction between DEVONthink paths and filesystem paths, the difference between replicate and duplicate, and the usage of database UUIDs for root operations are all important details that were lost.
My suggestions aim to restore this essential information while still keeping the descriptions concise.
| .describe( | ||
| "The DEVONthink location path of the record (e.g., '/Inbox/My Document'), NOT the filesystem path" | ||
| "DEVONthink location path of the record (e.g., '/Inbox/My Document')" | ||
| ), |
There was a problem hiding this comment.
The description for recordPath has been shortened, but removing the clarification 'NOT the filesystem path' might lead to user confusion. This was an important distinction. To maintain information density and prevent potential errors, I suggest re-adding this clarification.
| .describe( | |
| "The DEVONthink location path of the record (e.g., '/Inbox/My Document'), NOT the filesystem path" | |
| "DEVONthink location path of the record (e.g., '/Inbox/My Document')" | |
| ), | |
| .describe( | |
| "The DEVONthink location path of the record (e.g., '/Inbox/My Document'), NOT the filesystem path" | |
| ), |
|
@jules can you handle the gemini feedback |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| export const addTagsTool: Tool = { | ||
| name: "add_tags", | ||
| description: "Adds tags to a specific record in DEVONthink.", | ||
| name: "addTags", | ||
| title: "Add Tags", | ||
| description: "Adds tags to a DEVONthink record.\n\nExample:\n{\n \"uuid\": \"1234-5678-90AB-CDEF\",\n \"tags\": [\"important\", \"work\"]\n}", | ||
| inputSchema: zodToJsonSchema(AddTagsSchema) as ToolInput, |
There was a problem hiding this comment.
[P1] Remove unsupported title field from Tool objects
Every tool definition now sets a title property even though Tool from @modelcontextprotocol/sdk@1.0.1 only allows name, description, inputSchema, and run. Because the object literal is explicitly typed as Tool, tsc will reject the build with “Object literal may only specify known properties, and 'title' does not exist in type 'Tool'.” Running npm run build or npm run type-check will therefore fail until either the SDK is upgraded to a version that includes title or the extra field is removed.
Useful? React with 👍 / 👎.
|
@jules handle these points: Every tool definition now sets a title property even though Tool from @modelcontextprotocol/sdk@1.0.1 only allows name, description, inputSchema, and run. Because the object literal is explicitly typed as Tool, tsc will reject the build with “Object literal may only specify known properties, and 'title' does not exist in type 'Tool'.” Running npm run build or npm run type-check will therefore fail until either the SDK is upgraded to a version that includes title or the extra field is removed. also you renamed all tools from snake_case to camelCase. Rename the tool names back to what they were before. |
This change refactors all MCP tools to reduce token usage by shortening tool names and descriptions. It also adds a
titlefield to each tool and includes examples in the descriptions to maintain information density. The Zod schema descriptions for tool parameters have also been shortened. This addresses the user's request to make the tools more concise while still providing clear usage instructions.