Conversation
- New script: scripts/count-tokens.js - Uses js-tiktoken (cl100k_base) for accurate token counting - Spawns MCP server via JSON-RPC to get actual tools/list response - Shows per-tool breakdown (total, description, schema tokens) - Groups by category (Filesystem, Process, macOS AX, etc.) - Adds npm scripts: count-tokens, count-tokens:json - Baseline: 41 tools, 13,735 tokens (6.9% of 200K context)
Co-authored-by: codeant-ai[bot] <151821869+codeant-ai[bot]@users.noreply.github.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughThe PR adds token counting functionality for Desktop Commander MCP tool definitions. A new npm script spawns the MCP server, retrieves tool definitions via JSON-RPC, counts tokens using js-tiktoken, and outputs results in human-readable or JSON format. Includes related package.json updates. Changes
Sequence DiagramsequenceDiagram
participant User as User<br/>(CLI)
participant Script as count-tokens.js<br/>(Script)
participant Proc as MCP Server<br/>(Child Process)
participant Encoder as Tiktoken<br/>(Encoder)
participant Output as Output<br/>(Formatter)
User->>Script: npm run count-tokens [--json]
Script->>Proc: spawn MCP server (dist/index.js)
Script->>Proc: initialize JSON-RPC communication
Script->>Proc: send tools/list request
Proc-->>Script: JSON-RPC response with tools array
Script->>Script: parse server output lines as JSON
loop for each tool
Script->>Encoder: encode tool name, description, schema
Encoder-->>Script: token counts
Script->>Script: aggregate tokens per tool
end
Script->>Script: sort tools by token count (desc)
Script->>Script: calculate category groupings
Script->>Output: format output (human/JSON)
Output-->>User: display summary & breakdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Nitpicks 🔍
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
scripts/count-tokens.js (3)
140-140: Consider making the context window size configurable.The 200K context window is hardcoded in multiple places. Consider extracting it to a constant or making it configurable for users who want to compare against different model context windows.
♻️ Extract magic number to constant
+const CONTEXT_WINDOW_SIZE = 200_000; + async function main() { // ... - console.log(` Context window usage (200K): ${colors.yellow}${pct(grandTotal, 200000)}${colors.reset}`); + console.log(` Context window usage (${(CONTEXT_WINDOW_SIZE / 1000)}K): ${colors.yellow}${pct(grandTotal, CONTEXT_WINDOW_SIZE)}${colors.reset}`); // ... - contextWindowPct: (grandTotal / 200000 * 100).toFixed(2), + contextWindowPct: (grandTotal / CONTEXT_WINDOW_SIZE * 100).toFixed(2),Also applies to: 187-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/count-tokens.js` at line 140, The hardcoded 200000 context window in scripts/count-tokens.js (used where console.log prints pct(grandTotal, 200000) and the other occurrence around line 187) should be extracted to a single constant (e.g., CONTEXT_WINDOW) or made configurable via an environment variable / CLI flag; update the uses in the pct calls and any other comparisons to reference CONTEXT_WINDOW (and add a default value of 200000) so users can override it and to avoid duplicated magic numbers across the file.
65-77: Fixed timeouts may be fragile on slow systems.The hardcoded 500ms + 1500ms delays assume the server responds quickly. On slower systems or under load, this could cause spurious failures. Consider polling for the response message instead of using fixed delays, or at minimum, making the timeouts configurable via environment variables.
♻️ Example: Poll for response instead of fixed timeout
- setTimeout(() => { - const toolsRequest = { - jsonrpc: '2.0', id: 2, method: 'tools/list', params: {}, - }; - server.stdin.write(JSON.stringify(toolsRequest) + '\n'); - - setTimeout(() => { - server.kill(); - const resp = messages.find((m) => m.id === 2 && m.result); - if (!resp) return reject(new Error('No tools/list response')); - resolve(resp.result.tools); - }, 1500); - }, 500); + const TIMEOUT_MS = parseInt(process.env.COUNT_TOKENS_TIMEOUT, 10) || 5000; + const startTime = Date.now(); + + const checkForInit = setInterval(() => { + const initResp = messages.find((m) => m.id === 1 && m.result); + if (initResp) { + clearInterval(checkForInit); + const toolsRequest = { + jsonrpc: '2.0', id: 2, method: 'tools/list', params: {}, + }; + server.stdin.write(JSON.stringify(toolsRequest) + '\n'); + + const checkForTools = setInterval(() => { + const resp = messages.find((m) => m.id === 2 && m.result); + if (resp) { + clearInterval(checkForTools); + server.kill(); + resolve(resp.result.tools); + } else if (Date.now() - startTime > TIMEOUT_MS) { + clearInterval(checkForTools); + server.kill(); + reject(new Error('Timeout waiting for tools/list response')); + } + }, 100); + } else if (Date.now() - startTime > TIMEOUT_MS) { + clearInterval(checkForInit); + server.kill(); + reject(new Error('Timeout waiting for initialize response')); + } + }, 100);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/count-tokens.js` around lines 65 - 77, The current fixed delays around the tools/list request (the nested setTimeouts that write toolsRequest to server.stdin and then wait 1500ms before checking messages.find(m => m.id === 2 && m.result)) are fragile; replace them with a poll or wait-until loop that periodically checks messages for the response (id === 2) with a configurable overall timeout (via env var like TOOLS_LIST_TIMEOUT_MS) and only kills the server after the timeout elapses or the response is found; ensure you keep the same identifiers (toolsRequest, server.stdin.write, messages.find, server.kill) so callers remain clear and allow the polling interval and max wait to be tuned for slow systems.
53-53: Consider capturing stderr for better error diagnostics.Silently discarding stderr makes it harder to debug when the server fails to start. Consider logging it in non-JSON mode or including it in error messages.
♻️ Suggested improvement
- server.stderr.on('data', () => {}); // silence + let stderrOutput = ''; + server.stderr.on('data', (data) => { + stderrOutput += data.toString(); + });Then include
stderrOutputin the error message when rejecting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/count-tokens.js` at line 53, The current line server.stderr.on('data', () => {}); discards stderr and loses useful diagnostics; change it to accumulate stderr into a variable (e.g., stderrOutput) by collecting chunks on server.stderr.on('data', chunk => stderrOutput += chunk.toString()), log stderrOutput when running in non-JSON mode, and include stderrOutput in the error message when you reject/throw (where the code rejects the Promise that awaits server start) so failures surface the server stderr for debugging; keep the existing silence behavior for JSON mode if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/count-tokens.js`:
- Line 140: The hardcoded 200000 context window in scripts/count-tokens.js (used
where console.log prints pct(grandTotal, 200000) and the other occurrence around
line 187) should be extracted to a single constant (e.g., CONTEXT_WINDOW) or
made configurable via an environment variable / CLI flag; update the uses in the
pct calls and any other comparisons to reference CONTEXT_WINDOW (and add a
default value of 200000) so users can override it and to avoid duplicated magic
numbers across the file.
- Around line 65-77: The current fixed delays around the tools/list request (the
nested setTimeouts that write toolsRequest to server.stdin and then wait 1500ms
before checking messages.find(m => m.id === 2 && m.result)) are fragile; replace
them with a poll or wait-until loop that periodically checks messages for the
response (id === 2) with a configurable overall timeout (via env var like
TOOLS_LIST_TIMEOUT_MS) and only kills the server after the timeout elapses or
the response is found; ensure you keep the same identifiers (toolsRequest,
server.stdin.write, messages.find, server.kill) so callers remain clear and
allow the polling interval and max wait to be tuned for slow systems.
- Line 53: The current line server.stderr.on('data', () => {}); discards stderr
and loses useful diagnostics; change it to accumulate stderr into a variable
(e.g., stderrOutput) by collecting chunks on server.stderr.on('data', chunk =>
stderrOutput += chunk.toString()), log stderrOutput when running in non-JSON
mode, and include stderrOutput in the error message when you reject/throw (where
the code rejects the Promise that awaits server start) so failures surface the
server stderr for debugging; keep the existing silence behavior for JSON mode if
needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonscripts/count-tokens.js
| } | ||
| }); | ||
|
|
||
| server.stderr.on('data', () => {}); // silence |
There was a problem hiding this comment.
Suggestion: If the child process exits early or fails to spawn correctly, writes to server.stdin can emit an error event on the stdin stream; without an attached error handler this becomes an uncaught exception that crashes the script instead of cleanly rejecting the promise. [possible bug]
Severity Level: Major ⚠️
- ❌ count-tokens script crashes on child startup failures.
- ⚠️ No friendly error when MCP server cannot start.| server.stderr.on('data', () => {}); // silence | |
| server.stderr.on('data', () => {}); // silence | |
| server.stdin.on('error', (err) => reject(err)); |
Steps of Reproduction ✅
1. From the repo root, run `node scripts/count-tokens.js` without having built
`dist/index.js`, so the server entrypoint at `scripts/count-tokens.js:33` (`join(rootDir,
'dist', 'index.js')`) does not exist.
2. `main()` at `scripts/count-tokens.js:99-194` calls `extractToolsFromServer()` at line
110, which executes `spawn('node', [serverPath], { ... })` at lines 34-37; the child
process fails to start and emits an `'error'` event on the `ChildProcess` instance
(handled by `server.on('error', (e) => reject(e))` at line 79).
3. Regardless of the spawn failure, `extractToolsFromServer()` proceeds to call
`server.stdin.write(...)` at line 63 and again at line 69; because the child has already
errored/closed, these writes can emit an `'error'` event on `server.stdin`.
4. There is no `server.stdin.on('error', ...)` handler in the current code, so the
`'error'` event on `server.stdin` becomes an uncaught exception, terminating the entire
`node scripts/count-tokens.js` process rather than cleanly rejecting the promise and
flowing through the `main().catch(...)` handler at lines 217-220.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/count-tokens.js
**Line:** 53:53
**Comment:**
*Possible Bug: If the child process exits early or fails to spawn correctly, writes to `server.stdin` can emit an `error` event on the stdin stream; without an attached error handler this becomes an uncaught exception that crashes the script instead of cleanly rejecting the promise.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
CodeAnt AI finished reviewing your PR. |
CodeAnt-AI Description
Count MCP tool tokens from the running server and show per-tool and category breakdowns
What Changed
Impact
✅ Clearer token usage visibility for MCP tools✅ Fewer surprises from context window overruns✅ Easier audits of which tools consume the most LLM context💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Chores