Skip to content

feat(hotspots): roll up MCP server tools into one row per server#427

Open
willwashburn wants to merge 1 commit into
mainfrom
claude/issue-424-pr-ci-RfYJX
Open

feat(hotspots): roll up MCP server tools into one row per server#427
willwashburn wants to merge 1 commit into
mainfrom
claude/issue-424-pr-ci-RfYJX

Conversation

@willwashburn
Copy link
Copy Markdown
Member

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 every mcp__<server>__<tool> call into one row per server so you can ask "how much did relaycast cost me?" without manual filtering.

  • SDK (relayburn-sdk): new McpServerAggregation struct + aggregate_by_mcp_server(&[ToolAttribution]) -> Vec<_> in analyze/hotspots.rs. Skips any tool whose name doesn't split cleanly into mcp__<server>__<tool>. Sums call_count, initial_tokens, persistence_tokens, riding_turns, total_cost. Carries up to three representative tool basenames (cost desc, then name asc). Sort by total_cost desc, server asc as tiebreaker. Re-exported from analyze.rs and lib.rs.
  • HotspotsAttributionResult: new mcp_servers: Vec<McpServerAggregation> field, populated from the same attribution loop the existing file / bash / subagent rollups consume.
  • CLI (relayburn-cli): burn hotspots --json adds mcpServers after subagents; human renderer adds a Top MCP servers by cost section gated on a non-empty rollup.
  • Node facade (@relayburn/sdk): new HotspotsMcpServerRow interface, added to HotspotsAttributionResult.mcpServers. The napi-rs serde-json bridge already promotes callCount / ridingTurns to BigInt.
  • Changelogs: top-level + packages/sdk-node.

Test plan

  • New SDK unit test aggregates_by_mcp_server_groups_by_server_segment_and_sorts_by_cost covers: two MCP servers + non-MCP tools + malformed mcp__ 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 --workspace green (672 tests).
  • CLI golden snapshot updated (hotspots-json.stdout.txt adds "mcpServers": [] since the in-tree fixture has no MCP tools). BURN_GOLDEN=1 cargo test -p relayburn-cli --test golden green.
  • pnpm run test green (including @relayburn/sdk conformance test with napi build).
  • Type surface packages/sdk-node/src/index.d.ts reflects the new field.

Generated by Claude Code

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.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR implements a rollup feature for MCP server tool attribution in the hotspots analyzer. It introduces a new aggregation type that groups mcp__<server>__<tool> tool calls by server, sums their costs and token usage, ranks representative tools, and surfaces the results in both CLI human and JSON output and through the Node SDK.

Changes

MCP server rollup feature

Layer / File(s) Summary
Core data model and aggregation logic
crates/relayburn-sdk/src/analyze/hotspots.rs, crates/relayburn-sdk/src/analyze.rs, crates/relayburn-sdk/src/lib.rs
McpServerAggregation struct represents aggregated attribution per MCP server. parse_mcp_tool_name extracts server and tool from mcp__<server>__<tool> format. aggregate_by_mcp_server groups tool attributions by server, sums call counts/tokens/costs, computes top tools sorted by cost desc, and returns rows sorted by total cost desc. Unit tests validate server grouping, name parsing, and cost/token aggregation. New types re-exported through SDK public API.
Query result integration
crates/relayburn-sdk/src/query_verbs.rs
HotspotsAttributionResult extended with mcp_servers: Vec<McpServerAggregation> field. run_hotspots_attribution calls aggregate_by_mcp_server alongside existing bash/file/subagent aggregations. MCP servers computed and wired into both successful attribution payloads and refused-attribution empty fallbacks.
CLI presentation
crates/relayburn-cli/src/commands/hotspots.rs
JSON output includes new mcpServers field populated from aggregation results via mcp_server_to_json helper. Human renderer conditionally prints "Top MCP servers by cost" table with server name, call counts, token totals, cost, and top tools, formatted via mcp_server_row helper.
Node SDK types and documentation
packages/sdk-node/src/index.d.ts, CHANGELOG.md files, tests/fixtures/cli-golden/snapshots/hotspots-json.stdout.txt
HotspotsMcpServerRow TypeScript interface added to Node SDK. HotspotsAttributionResult extended with mcpServers field. Changelog entries in both Rust and Node SDKs document the new rollup capability. Golden test snapshot updated with empty mcpServers array.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • AgentWorkforce/burn#315: Extends the relayburn-cli hotspots command to render the new MCP server rollups in JSON and human output formats.
  • AgentWorkforce/burn#291: Introduced the foundational ToolAttribution and hotspots aggregation model that the MCP server rollup operates on.

🐰 In the hotspots so bright and clear,
MCP servers roll up near,
No more fifty tools to track—
Relaycast costs collapsed back!
One row reigns where many stood,
Budgets understood as they should. 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(hotspots): roll up MCP server tools into one row per server' clearly and specifically describes the main feature being added, matching the core changeset objective of aggregating MCP tools.
Description check ✅ Passed The description comprehensively details the MCP server rollup feature, implementation approach across SDK/CLI/Node packages, and test coverage—all directly related to the changeset.
Linked Issues check ✅ Passed The PR fully implements issue #424: new McpServerAggregation struct, aggregate_by_mcp_server function with proper grouping/sorting, HotspotsAttributionResult field addition, CLI JSON/human output support, Node facade types, and comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #424: SDK hotspots analysis, CLI hotspots display, Node type definitions, changelogs, and test fixtures—with no unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-424-pr-ci-RfYJX

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9b25822 and 5c7098c.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • crates/relayburn-cli/src/commands/hotspots.rs
  • crates/relayburn-sdk/src/analyze.rs
  • crates/relayburn-sdk/src/analyze/hotspots.rs
  • crates/relayburn-sdk/src/lib.rs
  • crates/relayburn-sdk/src/query_verbs.rs
  • packages/sdk-node/CHANGELOG.md
  • packages/sdk-node/src/index.d.ts
  • tests/fixtures/cli-golden/snapshots/hotspots-json.stdout.txt

Comment thread CHANGELOG.md
Comment on lines +9 to +13
- `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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
- `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.

Comment on lines +7 to +10
- `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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
- `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).

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hotspots: roll up MCP server tools (e.g. relaycast) into one row

2 participants