feat: collapse large MCP servers' tool rows into an expandable summary - #293
Conversation
Servers like GitHub expose 20+ individual mcp__github__* tools, each rendered as its own row in the MCP Tool Usage dashboard, dominating the card. Once a server exceeds TOOL_COLLAPSE_THRESHOLD (8) distinct methods, wrap the existing renderMethodRows breakdown in a native <details>/<summary> disclosure showing "N tools, TOTAL calls" instead -- no JS event wiring needed, and the native marker/triangle is the expand affordance. Servers at or under the threshold are unaffected. The <details> wrapper sits outside .methods (not inside it), so .methods' shared 3-column grid contract (#284) is unchanged in both the collapsed and expanded states. Closes #283 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNU12ST32LtQf2GPDPrZB5
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe MCP usage view now collapses per-server method lists with more than eight tools. The collapsed summary shows tool and aggregate call counts. Users can expand the summary to view detailed method rows. Source-containment tests verify the implementation. ChangesMCP tool grouping
Sequence Diagram(s)sequenceDiagram
participant renderServerCard
participant renderMethodsBlock
participant renderMethodRows
participant detailsElement
renderServerCard->>renderMethodsBlock: pass server method data
alt fewer than or equal to eight tools
renderMethodsBlock->>renderMethodRows: render expanded method rows
else more than eight tools
renderMethodsBlock->>detailsElement: render collapsed aggregate summary
detailsElement->>renderMethodRows: reveal detailed method rows
end
Merge Risk: ⚪ Minimal · up to The change only collapses large server tool lists behind a native disclosure while preserving existing rendering for smaller lists. Merge readiness risk is minimal, but the new regression test should verify that the threshold constant actually controls the collapsed-versus-direct rendering decision. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The PR implements grouped MCP tool rows, an expandable per-tool breakdown, an eight-tool threshold, and aggregate call counts for issue ✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
Comment |
|
Reopening to trigger CodeRabbit review with needs-review already applied. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fdb13c1a-6d14-4e06-a1d2-8b94d93ece48
📒 Files selected for processing (2)
src/claude_prospector/static/views/mcp-usage.jstests/test_mcp_usage_view_tool_grouping.py
Included review availability: 6 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.
| named_constant_pattern = re.compile( | ||
| r"const\s+\w*(?:THRESHOLD|COLLAPSE|MAX)\w*\s*=\s*\d+", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Require the threshold constant to control a render branch.
Lines 129-132 accept an unused threshold constant. The test can pass when no method-count condition selects between direct and collapsed rendering. Assert that the declared threshold is used in a .length comparison and that the under-threshold branch returns the direct method markup.
Proposed test direction
- named_constant_pattern = re.compile(
- r"const\s+\w*(?:THRESHOLD|COLLAPSE|MAX)\w*\s*=\s*\d+",
- re.IGNORECASE,
- )
+ threshold_match = re.search(
+ r"const\s+(?P<name>\w*(?:THRESHOLD|COLLAPSE|MAX)\w*)\s*=\s*\d+",
+ content,
+ re.IGNORECASE,
+ )
+ assert threshold_match
+ threshold_name = re.escape(threshold_match.group("name"))
+ assert re.search(
+ rf"\b\w+\.length\s*(?:>=|>|<=|<)\s*{threshold_name}\b",
+ content,
+ )CodeRabbit flagged that the collapsed <details> summary row only
aggregated call counts ("N tools, TOTAL calls"), not tokens, even
though issue #283 asks to aggregate "calls/tokens" and
info.by_method_tokens was already available. renderMethodsBlock now
also reduces by_method_tokens (guarded for its absence when
--track-mcp-call-sizes was off) and appends a "~N tokens" clause to
the summary text when present.
Includes the accompanying frozen test additions to
tests/test_mcp_usage_view_tool_grouping.py
(TestCollapsedSummaryAggregatesTokensAlongsideCalls).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNU12ST32LtQf2GPDPrZB5
|
Addressed both items from CodeRabbit's review in
All 5 CI gates re-verified green after the fix (CodeRabbit shows "incremental reviews are disabled" on the follow-up push per this repo's config, so no new automated pass — verification was done manually, see commit message for full gate output). 🤖 Generated by Claude Code on behalf of @cbeaulieu-gt |
…r-tools # Conflicts: # src/claude_prospector/static/views/mcp-usage.js
1 participant
Summary
Servers like GitHub expose 20+ individual
mcp__github__*tools, each rendered as its own row in the MCP Tool Usage dashboard view, dominating the card and making cross-server comparison hard.TOOL_COLLAPSE_THRESHOLD = 8(named constant). Once a server'sby_methodentry count exceeds it, the existing per-method breakdown (renderMethodRows, unchanged) is wrapped in a native<details><summary>N tools, TOTAL calls</summary>...</details>disclosure instead of rendering every row directly.TOTAL callsisObject.values(by_method).reduce(...), which is always equal to the server card's existing "Total calls" stat by construction (verified againstaggregator.py's single-loop accumulation ofserver_calls/server_methods), so the summary can never diverge from the stat above it.<details>/<summary>— no JS event wiring, no custom disclosure styling (kept the browser's default marker as the only "this is clickable" affordance, persimplicity-first).<details>wrapper sits outside.methods, so.methods' shared 3-column CSS grid contract (issue Usage count / est. tokens columns misaligned in dashboard cells #284) is unchanged in both collapsed and expanded states.Built test-first:
tests/test_mcp_usage_view_tool_grouping.pywas written and confirmed red before any implementation, then the implementation went green against it without editing the frozen tests.Closes #283
Test plan
pytest tests/test_mcp_usage_view_tool_grouping.py tests/test_mcp_usage_view_column_alignment.py tests/test_mcp_usage_view.py tests/test_mcp_usage_view_guid_filter.py tests/test_mcp_usage_view_zero_call_filter.py tests/test_mcp_usage_view_cost_proxy.py -v— 49 passed, no regressions (especially the Usage count / est. tokens columns misaligned in dashboard cells #284 3-column grid contract)pytest tests/(full suite) — 920 passed, 1 failed (pre-existing, unrelated date-boundary flake:test_tool_usage.py::TestWindowBoundsToAnchor::test_days_only_default_path_is_unaffected), 2 skippedruff check .— cleanruff format --check .— clean🤖 Generated by Claude Code on behalf of @cbeaulieu-gt
Summary by CodeRabbit
New Features
Style
Tests