Skip to content

refactor: reduce complexity of analyze_all in src/audit/analyzers/mcp.rs#1090

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

refactor: reduce complexity of analyze_all in src/audit/analyzers/mcp.rs#1090
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-mcp-analyzer-cd647278a306393d

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 (Clippy threshold=10). The function contained a deeply-nested structure:

for path in file_paths          // +1
  for line in contents.lines()  // +1
    match serde_json::from_str  // +1
    let Some(event_kind) ...    // +1
    match event_kind.as_str()   // +1
      "tool_call" arm           // +1
        if !server.is_empty()   // +1
        if let Some(tool)       // +1
      "tool_error" arm          // ...and so on

On top of the parsing loop, the function also inlined the directory validation guard and 40+ lines of server-rollup building, making it hard to read and test each concern in isolation.

What changed

New EventAccumulators struct + impl

Groups the five accumulator variables (saw_recognizable_event, per_tool, observed_servers, server_error_counts, failures) into a struct. The impl provides a process_event dispatcher and four focused handlers:

  • on_tool_call — increments call count, tracks server/tool sizes
  • on_tool_error — increments error count, appends to failure list
  • on_server_error — increments server error counter, appends to failure list
  • on_server_lifecycle — records the server name

Four extracted helper functions

Helper Replaces
validate_logs_dir_exists Inline match tokio::fs::metadata(...) guard at the top of analyze_all
process_log_contents The inner for line in contents.lines() loop
build_tool_summaries The tool-list construction and sort
build_server_stats The three-pass server-rollup construction and sort

analyze_all after refactor

async fn analyze_all(mcpg_logs_dir: &Path) -> Result<AnalyzeAllResult> {
    if !validate_logs_dir_exists(mcpg_logs_dir).await? {
        return Ok(AnalyzeAllResult::default());
    }
    let file_paths = read_log_file_paths(mcpg_logs_dir).await?;
    let mut acc = EventAccumulators::default();
    for path in file_paths {
        let contents = tokio::fs::read_to_string(&path).await...?;
        process_log_contents(&mut acc, &contents);
    }
    if !acc.saw_recognizable_event {
        return Ok(AnalyzeAllResult::default());
    }
    let tools = build_tool_summaries(&acc.per_tool);
    let servers = build_server_stats(acc.observed_servers, &acc.per_tool, acc.server_error_counts);
    Ok(AnalyzeAllResult { tool_usage: Some(...), server_health: Some(...), failures: acc.failures })
}

Before / after complexity

Metric Before After
analyze_all cognitive complexity 19 below 10
Clippy warnings at threshold=10 for this file 1 0

Verification

  • All 2062 tests pass (cargo test --bin ado-aw)
  • All 7 audit::analyzers::mcp unit tests pass
  • cargo clippy --all-targets --all-features — no new warnings
  • Zero clippy::cognitive_complexity warnings for src/audit/analyzers/mcp.rs at threshold=10

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 · 605.1 AIC · ⌖ 39.5 AIC · ⊞ 36.2K ·

Extract EventAccumulators struct with per-event handler methods
(on_tool_call, on_tool_error, on_server_error, on_server_lifecycle)
to replace a deeply-nested for+for+match inside analyze_all.

Also extract four helpers:
- validate_logs_dir_exists   — replaces the inline metadata guard
- process_log_contents       — handles the per-line parse/dispatch loop
- build_tool_summaries       — builds and sorts the MCPToolSummary list
- build_server_stats         — builds and sorts the MCPServerStats list

analyze_all is now a linear 20-line orchestration function.
Cognitive complexity: 19 → below 10 (zero clippy::cognitive_complexity
warnings at threshold=10). All 2062 tests pass.

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