Uh oh!
There was an error while loading. Please reload this page.
feat(hotspots): roll up MCP server tools into one row per server - #427
Conversation
Closes#424. `burn hotspots` already broke down per-tool cost, but a chatty MCP server (relaycast, github, ...) showed up as N independent rows — none individually expensive, collectively a hidden hotspot. The SDK now exposes `McpServerAggregation` + `aggregate_by_mcp_server`, which group any `mcp__<server>__<tool>` attribution by `<server>`, sum calls / tokens / ride-turns / cost, and carry the top three representative tool basenames (cost desc, name asc). The CLI surfaces the rollup as `mcpServers` in `--json` and renders a `Top MCP servers by cost` table when non-empty.
📝 WalkthroughWalkthroughThis PR implements a rollup feature for MCP server tool attribution in the hotspots analyzer. It introduces a new aggregation type that groups ChangesMCP server rollup feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 9-13: The Unreleased changelog entry ending with "(`#424`)" should
drop the PR/issue marker: edit the CHANGELOG.md entry for "burn hotspots" (the
Unreleased root) and remove the trailing " (`#424`)" so the line ends after
"`mcpServers` in `--json`." and no PR/issue/reference remains; keep the rest of
the text unchanged.
In `@packages/sdk-node/CHANGELOG.md`:
- Around line 7-10: Update the Unreleased changelog bullet in CHANGELOG.md by
removing the trailing issue/PR reference "(`#424`)" from the hotspots() entry;
keep the text and impact-first phrasing intact and ensure the entry still
mentions the new mcpServers field and description of per-server rollup of
mcp__<server>__<tool> calls (i.e., only delete the issue tag, do not change
function name hotspots() or field name mcpServers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf01fdf1-bcca-40c3-b3e0-4686d7c14cb0
📒 Files selected for processing (9)
CHANGELOG.mdcrates/relayburn-cli/src/commands/hotspots.rscrates/relayburn-sdk/src/analyze.rscrates/relayburn-sdk/src/analyze/hotspots.rscrates/relayburn-sdk/src/lib.rscrates/relayburn-sdk/src/query_verbs.rspackages/sdk-node/CHANGELOG.mdpackages/sdk-node/src/index.d.tstests/fixtures/cli-golden/snapshots/hotspots-json.stdout.txt
| - `burn hotspots`: new MCP-server rollup that collapses every | ||
| `mcp__<server>__<tool>` tool call into one row per server, so a chatty | ||
| MCP server (e.g. relaycast) shows up as a single line instead of 50+. | ||
| Surfaced in the human renderer (when non-empty) and as `mcpServers` | ||
| in `--json`. (#424) |
There was a problem hiding this comment.
Drop the PR/issue marker from the root Unreleased entry.
The impact text is good; just remove (#424) to match changelog policy.
Suggested edit
- `burn hotspots`: new MCP-server rollup that collapses every
`mcp__<server>__<tool>` tool call into one row per server, so a chatty
MCP server (e.g. relaycast) shows up as a single line instead of 50+.
Surfaced in the human renderer (when non-empty) and as `mcpServers`
- in `--json`. (`#424`)+ in `--json`.As per coding guidelines, changelog entries should “Drop issue/PR links, internal review notes, implementation backstory…”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -`burn hotspots`: new MCP-server rollup that collapses every | |
| `mcp__<server>__<tool>` tool call into one row per server, so a chatty | |
| MCP server (e.g. relaycast) shows up as a single line instead of 50+. | |
| Surfaced in the human renderer (when non-empty) and as `mcpServers` | |
| in `--json`. (#424) | |
| -`burn hotspots`: new MCP-server rollup that collapses every | |
| `mcp__<server>__<tool>` tool call into one row per server, so a chatty | |
| MCP server (e.g. relaycast) shows up as a single line instead of 50+. | |
| Surfaced in the human renderer (when non-empty) and as `mcpServers` | |
| in `--json`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 9 - 13, The Unreleased changelog entry ending with
"(`#424`)" should drop the PR/issue marker: edit the CHANGELOG.md entry for "burn
hotspots" (the Unreleased root) and remove the trailing " (`#424`)" so the line
ends after "`mcpServers` in `--json`." and no PR/issue/reference remains; keep
the rest of the text unchanged.
| - `hotspots()` attribution result carries a new `mcpServers` field — | ||
| per-server rollup of `mcp__<server>__<tool>` tool calls with summed | ||
| cost/tokens/ride-turns and top representative tool basenames. (#424) | ||
There was a problem hiding this comment.
Remove the issue reference from the Unreleased bullet.
Keep the entry impact-first, but drop the trailing (#424) per changelog style rules.
Suggested edit
-- `hotspots()` attribution result carries a new `mcpServers` field —- per-server rollup of `mcp__<server>__<tool>` tool calls with summed- cost/tokens/ride-turns and top representative tool basenames. (`#424`)+- `hotspots()` attribution result carries a new `mcpServers` field —+ per-server rollup of `mcp__<server>__<tool>` tool calls with summed+ cost/tokens/ride-turns and top representative tool basenames.As per coding guidelines, changelog entries should “Drop issue/PR links, internal review notes, implementation backstory…”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -`hotspots()` attribution result carries a new `mcpServers` field — | |
| per-server rollup of `mcp__<server>__<tool>` tool calls with summed | |
| cost/tokens/ride-turns and top representative tool basenames. (#424) | |
| -`hotspots()` attribution result carries a new `mcpServers` field — | |
| per-server rollup of `mcp__<server>__<tool>` tool calls with summed | |
| cost/tokens/ride-turns and top representative tool basenames. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/sdk-node/CHANGELOG.md` around lines 7 - 10, Update the Unreleased
changelog bullet in CHANGELOG.md by removing the trailing issue/PR reference
"(`#424`)" from the hotspots() entry; keep the text and impact-first phrasing
intact and ensure the entry still mentions the new mcpServers field and
description of per-server rollup of mcp__<server>__<tool> calls (i.e., only
delete the issue tag, do not change function name hotspots() or field name
mcpServers).
Uh oh!
There was an error while loading. Please reload this page.
Main released 2.10.0 (hotspots MCP rollup #424), 2.10.1 (#427 + pending-stamp refactor #428), and the workspace-wide dead_code/ unused_imports cleanup #429. The latter removed ingest_claude_projects from the public re-export and switched orchestration_tests to `crate::ingest::ingest::*` deep imports. Conflict resolution: - crates/relayburn-sdk/src/ingest.rs: keep main's narrowed re-export (no ingest_claude_projects) and add ingest_claude_transcript_path. - crates/relayburn-sdk/src/ingest/orchestration_tests.rs: keep main's deep imports and add ingest_claude_transcript_path next to its sibling verbs. - CHANGELOG.md: slot this branch's items under [Unreleased] above main's new 2.10.0 release section. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
Summary
Closes#424. Adds a new MCP-server rollup to
burn hotspots. A chatty MCP server (relaycast with 50+ tools, the GitHub server, etc.) used to spread its cost across N per-tool rows where none individually looked expensive. The rollup now collapses everymcp__<server>__<tool>call into one row per server so you can ask "how much did relaycast cost me?" without manual filtering.relayburn-sdk): newMcpServerAggregationstruct +aggregate_by_mcp_server(&[ToolAttribution]) -> Vec<_>inanalyze/hotspots.rs. Skips any tool whose name doesn't split cleanly intomcp__<server>__<tool>. Sumscall_count,initial_tokens,persistence_tokens,riding_turns,total_cost. Carries up to three representative tool basenames (cost desc, then name asc). Sort bytotal_costdesc,serverasc as tiebreaker. Re-exported fromanalyze.rsandlib.rs.mcp_servers: Vec<McpServerAggregation>field, populated from the same attribution loop the existing file / bash / subagent rollups consume.relayburn-cli):burn hotspots --jsonaddsmcpServersaftersubagents; human renderer adds aTop MCP servers by costsection gated on a non-empty rollup.@relayburn/sdk): newHotspotsMcpServerRowinterface, added toHotspotsAttributionResult.mcpServers. The napi-rs serde-json bridge already promotescallCount/ridingTurnstoBigInt.packages/sdk-node.Test plan
aggregates_by_mcp_server_groups_by_server_segment_and_sorts_by_costcovers: two MCP servers + non-MCP tools + malformedmcp__names (empty server / empty tool / missing separator), verifies the non-MCP and malformed rows are skipped, costs sum, top_tools are cost-sorted, and server order is total_cost desc.cargo test --workspacegreen (672 tests).hotspots-json.stdout.txtadds"mcpServers": []since the in-tree fixture has no MCP tools).BURN_GOLDEN=1 cargo test -p relayburn-cli --test goldengreen.pnpm run testgreen (including@relayburn/sdkconformance test with napi build).packages/sdk-node/src/index.d.tsreflects the new field.Generated by Claude Code