Skip to content

refactor(audit): reduce complexity of analyze_all in mcp.rs#1167

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-mcp-analyzer-v2-7ac65b39cf894e62
Draft

refactor(audit): reduce complexity of analyze_all in mcp.rs#1167
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-mcp-analyzer-v2-7ac65b39cf894e62

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

analyze_all in src/audit/analyzers/mcp.rs had a cognitive complexity of 19 (threshold: 15). The function combined:

  • Directory validation with a 3-arm match
  • A nested double for loop (files → lines)
  • JSON parsing and event dispatch via a 4-arm match (tool_call / tool_error / server_error / server_start|stop), each with several if guards
  • Two post-loop phases building tool summaries and server health rollups (3 more loops + inline ternary for error_rate)

All of this lived in one ~200-line function with up to 5 levels of nesting.

What changed

Introduced EventAccumulators — a struct that owns the accumulation state (per_tool, observed_servers, server_error_counts, failures, saw_recognizable_event) and exposes four focused methods:

Method Responsibility
on_tool_call(&Value) Register a tool call in per-tool stats and observed servers
on_tool_error(Value) Register a tool error, update stats, record failure report
on_server_error(Value) Update server error counts, record failure report
on_server_lifecycle(&Value) Register server start/stop in observed servers
process_event(&str, Value) One-line dispatch to the above four methods

Extracted four free functions:

  • ensure_mcpg_logs_dir_exists — directory validation (3-arm match with early-return semantics)
  • process_log_files — file reading + line dispatch loop
  • build_tool_summaries — constructs and sorts Vec<MCPToolSummary> from accumulated state
  • build_server_health_list — builds server rollups from three accumulation maps and computes error_rate/unreliable

analyze_all is now a 6-line orchestrator with zero nesting of its own.

Before / After

Metric Before After
analyze_all cognitive complexity 19 < 15 (zero Clippy warnings at threshold=15)
Lines in analyze_all ~200 ~16
Max nesting depth 5 1

Verification

  • cargo build — clean
  • cargo test — all tests pass (including the MCP analyzer unit tests in the same file)
  • cargo clippy --all-targets --all-features — clean
  • cargo clippy ... -W clippy::cognitive_complexity with threshold=15 — zero findings for src/audit/analyzers/mcp.rs

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com
  • spsprodweu4.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"
    - "spsprodweu4.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 536.7 AIC · ⌖ 61.4 AIC · ⊞ 35.9K ·

Extract EventAccumulators struct with on_tool_call/on_tool_error/
on_server_error/on_server_lifecycle methods to replace the large
nested match block. Move directory validation into
ensure_mcpg_logs_dir_exists, file-reading+dispatch into
process_log_files, and result-building into build_tool_summaries
and build_server_health_list.

analyze_all is now a 6-line orchestrator; cognitive complexity drops
from 19 to below the 15 threshold.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

0 participants