Skip to content

Refactor: Extract duplicate log metrics finalization logic - #4105

Merged
pelikhan merged 4 commits into
mainfrom
copilot/refactor-duplicate-log-metrics
Nov 16, 2025
Merged

Refactor: Extract duplicate log metrics finalization logic#4105
pelikhan merged 4 commits into
mainfrom
copilot/refactor-duplicate-log-metrics

Conversation

Copilot AI commented Nov 15, 2025

Copy link
Copy Markdown
Contributor

Three engine parsers (Copilot, Codex, Claude) duplicated ~15 lines of metrics finalization: appending sequences, converting tool call maps to sorted slices, counting errors via pattern matching.

Changes

  • Added FinalizeToolMetrics() in metrics.go
    Handles full finalization (sequences, token usage, turns, sorting, error patterns). Used by Copilot and Codex.

  • Added FinalizeToolCallsAndSequence() in metrics.go
    Lightweight variant for Claude, which extracts token/turns from result entries rather than accumulating during parse.

  • Updated engine parsers
    Replaced 25-28 lines per engine with single function calls.

Before

// Copilot, Codex: Similar 25-line blocks
if len(currentSequence) > 0 {
    metrics.ToolSequences = append(metrics.ToolSequences, currentSequence)
}
metrics.TokenUsage = maxTokenUsage
metrics.Turns = turns
for _, toolInfo := range toolCallMap {
    metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo)
}
sort.Slice(metrics.ToolCalls, func(i, j int) bool {
    return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name
})
errorPatterns := e.GetErrorPatterns()
if len(errorPatterns) > 0 {
    metrics.Errors = CountErrorsAndWarningsWithPatterns(logContent, errorPatterns)
}

After

// Copilot, Codex
FinalizeToolMetrics(&metrics, toolCallMap, currentSequence, turns, tokenUsage, logContent, e.GetErrorPatterns())

// Claude
FinalizeToolCallsAndSequence(&metrics, toolCallMap, currentSequence)

Impact

  • Code reduction: 54 lines across engines
  • Tests: 9 cases covering edge cases, sorting, error patterns
  • Pattern established: Clear template for future engines
Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected: Log Metrics Finalization Across Engines</issue_title>
<issue_description>Analysis of commit 0dffa81

Assignee: @copilot

Summary

The engine-specific ParseLogMetrics implementations all repeat the same block that finalizes tool call metrics (appending the last sequence, copying the tool call map into a slice, sorting it, and counting errors). Copilot, Codex, and Claude pipelines each copy this logic verbatim with only variable name changes.

Duplication Details

Pattern: Repeated tool call finalization logic in log parsers

  • Severity: Medium
  • Occurrences: 3
  • Locations:
    • pkg/workflow/copilot_engine.go:492
    • pkg/workflow/codex_engine.go:407
    • pkg/workflow/claude_logs.go:340
  • Code Sample:
    if len(currentSequence) > 0 {
        metrics.ToolSequences = append(metrics.ToolSequences, currentSequence)
    }
    metrics.TokenUsage = maxTokenUsage
    metrics.Turns = turns
    for _, toolInfo := range toolCallMap {
        metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo)
    }
    sort.Slice(metrics.ToolCalls, func(i, j int) bool {
        return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name
    })
    errorPatterns := e.GetErrorPatterns()
    if len(errorPatterns) > 0 {
        metrics.Errors = CountErrorsAndWarningsWithPatterns(logContent, errorPatterns)
    }

Impact Analysis

  • Maintainability: Updates to how sequences, sorting, or error aggregation work must be repeated in three places, increasing the chance of drift when adding new engines.
  • Bug Risk: Any bug fixes to the metric finalization (e.g., changing how warnings are counted) must be manually mirrored everywhere; missing an occurrence yields inconsistent telemetry.
  • Code Bloat: The repeated blocks add ~15 lines per engine and make the already large log parsing routines harder to scan.

Refactoring Recommendations

  1. Extract shared helper

    • Move the duplication into a helper such as finalizeToolMetrics(metrics *LogMetrics, toolCallMap map[string]*ToolCallInfo, currentSequence []string, turns int, tokenUsage int, logContent string, getPatterns func() []ErrorPattern).
    • Estimated effort: Medium (1-2h) because the helper can live in pkg/workflow/log_parser.go and accept function pointers for engine-specific hooks.
    • Benefits: Single source of truth for tool sequencing and error aggregation.
  2. Standardize sequence tracking

    • Consider wrapping the shared state (tool call map, current sequence, turns, token totals) in a struct so engines update fields but delegate finalization to one method.
    • Benefits: Simplifies adding new engines like Claude variants without extra boilerplate.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 3
  • Detection Method: Serena semantic code analysis
  • Commit: 0dffa81
  • Analysis Date: 2025-11-15T21:06:44Z

AI generated by Duplicate Code Detector</issue_description>

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


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 15, 2025
Copilot AI and others added 2 commits November 15, 2025 23:43
- Add FinalizeToolMetrics() in metrics.go for full finalization (Copilot, Codex)
- Add FinalizeToolCallsAndSequence() in metrics.go for tool call finalization (Claude)
- Update Copilot engine to use FinalizeToolMetrics()
- Update Codex engine to use FinalizeToolMetrics()
- Update Claude engine to use FinalizeToolCallsAndSequence()
- Remove unused sort import from claude_logs.go
- Add comprehensive tests for both helper functions
- All existing tests pass without modification

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate log metrics finalization logic Refactor: Extract duplicate log metrics finalization logic Nov 15, 2025
Copilot AI requested a review from pelikhan November 15, 2025 23:50
@pelikhan
pelikhan marked this pull request as ready for review November 16, 2025 00:04
Copilot AI review requested due to automatic review settings November 16, 2025 00:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors duplicate log metrics finalization logic that was repeated across three engine parsers (Copilot, Codex, and Claude). The refactoring extracts ~15 lines of identical finalization code (appending sequences, converting tool call maps to sorted slices, counting errors via pattern matching) into two reusable helper functions in metrics.go.

Key changes:

  • Adds FinalizeToolMetrics() for full finalization (used by Copilot and Codex engines)
  • Adds FinalizeToolCallsAndSequence() as a lightweight variant (used by Claude engine which manages tokens/turns separately)
  • Replaces 25-28 lines per engine with single function calls

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/workflow/metrics.go Adds two new helper functions for metrics finalization and imports sort package
pkg/workflow/metrics_test.go Adds comprehensive test coverage with 9 test cases across 2 test functions
pkg/workflow/copilot_engine.go Replaces 25 lines of finalization logic with single FinalizeToolMetrics() call
pkg/workflow/codex_engine.go Replaces 25 lines of finalization logic with single FinalizeToolMetrics() call
pkg/workflow/claude_logs.go Replaces 18 lines of finalization logic with FinalizeToolCallsAndSequence() call and removes unused sort import

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

Comment thread pkg/workflow/metrics.go Outdated
Comment thread pkg/workflow/metrics.go
Comment thread pkg/workflow/metrics.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pelikhan
pelikhan merged commit 30f9dbd into main Nov 16, 2025
9 checks passed
@pelikhan
pelikhan deleted the copilot/refactor-duplicate-log-metrics branch November 16, 2025 00:09
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.

[duplicate-code] 🔍 Duplicate Code Detected: Log Metrics Finalization Across Engines

3 participants