Refactor: Extract generic aggregation helper to eliminate duplicate code in logs report

[Copilot]

buildMissingToolsSummary and buildMCPFailuresSummary in pkg/cli/logs_report.go contained ~47 lines of identical aggregation logic per function, differing only in struct types and field names.

Changes

• Added aggregateSummaryItems[TItem, TSummary] generic helper (38 lines)

  • Handles common pattern: group by key, count occurrences, deduplicate workflows, collect run IDs
  • Takes 5 function parameters for type-specific extraction, keying, creation, updating, and finalization

• Refactored both functions to use generic helper

  • Reduced each from 74 to 27 lines (64% per function)
  • Maintains all original behavior including FirstReason field for missing tools

• Added TestAggregateSummaryItems

  • Validates aggregation, deduplication, and finalization behavior

• Merged latest changes from main

  • Resolved conflicts in test file by keeping both test suites
  • Complementary aggregateDomainStats helper from main now coexists for domain aggregation
  • Both helper functions serve different aggregation purposes

Example

Before:

func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary {
    toolSummary := make(map[string]*MissingToolSummary)
    for _, pr := range processedRuns {
        for _, tool := range pr.MissingTools {
            if summary, exists := toolSummary[tool.Tool]; exists {
                summary.Count++
                summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName)
                summary.RunIDs = append(summary.RunIDs, tool.RunID)
            } else {
                toolSummary[tool.Tool] = &MissingToolSummary{...}
            }
        }
    }
    // ... 40 more lines
}

After:

func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary {
    result := aggregateSummaryItems(
        processedRuns,
        func(pr ProcessedRun) []MissingToolReport { return pr.MissingTools },
        func(tool MissingToolReport) string { return tool.Tool },
        func(tool MissingToolReport) *MissingToolSummary { return &MissingToolSummary{...} },
        func(summary *MissingToolSummary, tool MissingToolReport) { /* update */ },
        func(summary *MissingToolSummary) { /* finalize display fields */ },
    )
    // ... sort and return
}

Impact: 56 lines eliminated (38% reduction). Future summary types can reuse the pattern. Successfully integrated with complementary refactoring from main branch.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected: Logs Report Tool Failure Summaries</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Logs Report Tool Failure Summaries

Analysis of commit 837cf5f

Assignee: @copilot

Summary

The helper functions buildMissingToolsSummary and buildMCPFailuresSummary in pkg/cli/logs_report.go share the same aggregation flow when turning processedRuns into summary structs, differing only by target type and field names.

Duplication Details

Pattern: Duplicate aggregation for missing tools vs. MCP failures

• Severity: Medium
• Occurrences: 2
• Locations:
  • pkg/cli/logs_report.go:295
  • pkg/cli/logs_report.go:334
• Code Sample:

  summaryMap := make(map[string]*T)
  
  for _, pr := range processedRuns {
      for _, item := range sourceSlice(pr) {
          if summary, exists := summaryMap[item.Key]; exists {
              summary.Count++
              summary.Workflows = addUniqueWorkflow(summary.Workflows, item.WorkflowName)
              summary.RunIDs = append(summary.RunIDs, item.RunID)
          } else {
              summaryMap[item.Key] = &T{
                  Key:       item.Key,
                  Count:     1,
                  Workflows: []string{item.WorkflowName},
                  RunIDs:    []int64{item.RunID},
              }
          }
      }
  }

Impact Analysis

• Maintainability: Any display change (e.g., new fields or sorting tweaks) must be applied twice.
• Bug Risk: Diverging defaults or forgetting to update one branch can lead to inconsistent CLI output.
• Code Bloat: ~35 lines repeated back-to-back obscure the differences between summaries.

Refactoring Recommendations

1. Generic summarizer helper

   • Introduce a parameterized helper that takes key selectors and constructors for the summary struct.
   • Estimated effort: Medium (~2 hours)
   • Benefits: Consolidates repeated loops, makes future summary types easier to add.

2. Leverage typed adapters

   • Define small adapter interfaces (e.g., logSummaryItem) so both cases can share the aggregation pipeline while providing type-specific assignments.
   • Estimated effort: Medium
   • Benefits: Makes intent clearer and removes map boilerplate duplication.

Implementation Checklist

• Review duplication findings
• Prioritize refactoring tasks
• Create refactoring plan
• Implement changes
• Update tests
• Verify no functionality broken

Analysis Metadata

• Analyzed Files: 1
• Detection Method: Serena semantic code analysis
• Commit: 837cf5f
• Analysis Date: 2025-11-11T02:21:46Z

AI generated by Duplicate Code Detector</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [duplicate-code] 🔍 Duplicate Code Detected: Logs Report Tool Failure Summaries #3584

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull Request Overview

This PR successfully refactors duplicate aggregation logic in the logs report functionality by introducing a generic helper function. The refactoring eliminates ~94 lines of duplicate code (47 lines in each of two functions) while maintaining all original behavior.

Key Changes

• Added aggregateSummaryItems generic helper function - Provides a reusable pattern for aggregating items by key, counting occurrences, deduplicating workflows, and collecting run IDs using function parameters for type-specific behavior
• Refactored buildMissingToolsSummary and buildMCPFailuresSummary - Both functions now use the generic helper, reducing each from ~74 lines to ~40 lines (46% reduction per function)
• Added comprehensive test coverage - New TestAggregateSummaryItems validates the generic helper's aggregation, deduplication, and finalization behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/cli/logs_report.go	Introduced generic aggregateSummaryItems helper function and refactored two existing summary builder functions to use it, eliminating duplicate aggregation logic
pkg/cli/logs_report_test.go	Added TestAggregateSummaryItems to validate the new generic helper function's behavior with actual data types

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] The test calls aggregateSummaryItems directly, but since this is a generic helper function that's not exported (starts with lowercase a), it won't be accessible outside the package. This test will only work because it's in the same package (cli).

While this works for now, consider whether aggregateSummaryItems should be:

1. Exported (renamed to AggregateSummaryItems) if it's intended to be reusable across packages
2. Kept private if it's only for internal use within this package (current approach)

The current approach is fine for internal testing, but the naming and visibility should be intentional.

[Copilot]

The function is intentionally kept private (unexported) as it's an internal helper specifically designed for use within the cli package. The current approach ensures encapsulation - the generic aggregation pattern is not exposed as a public API since it's implementation-specific to the logs report functionality. Exporting it would create an unnecessary public interface that we'd need to maintain for backward compatibility.

[pelikhan]

@copilot merge main

[Copilot]

@copilot merge main

Done in 8f3bd4a. Successfully merged main branch and resolved conflicts in pkg/cli/logs_report_test.go by keeping both test suites. Both helper functions (aggregateSummaryItems and aggregateDomainStats) now coexist and serve complementary purposes. All tests pass.