diff --git a/pkg/workflow/claude_logs.go b/pkg/workflow/claude_logs.go index 72bc3403e36..933115a30e0 100644 --- a/pkg/workflow/claude_logs.go +++ b/pkg/workflow/claude_logs.go @@ -3,7 +3,6 @@ package workflow import ( "encoding/json" "fmt" - "sort" "strings" "time" @@ -337,10 +336,8 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe } } - // Add the complete sequence if we found any tool calls - if len(currentSequence) > 0 { - metrics.ToolSequences = append(metrics.ToolSequences, currentSequence) - } + // Finalize tool calls and sequences using shared helper + FinalizeToolCallsAndSequence(&metrics, toolCallMap, currentSequence) if verbose && len(metrics.ToolSequences) > 0 { totalTools := 0 @@ -351,16 +348,6 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe len(metrics.ToolSequences), totalTools) } - // Convert tool call map to slice - for _, toolInfo := range toolCallMap { - metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo) - } - - // Sort tool calls by name for consistent output - sort.Slice(metrics.ToolCalls, func(i, j int) bool { - return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name - }) - return metrics } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 7bdf02ec486..0f20a1affab 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -404,29 +404,8 @@ func (e *CodexEngine) ParseLogMetrics(logContent string, verbose bool) LogMetric // Basic processing - error/warning counting moved to end of function } - // Add final sequence if any - if len(currentSequence) > 0 { - metrics.ToolSequences = append(metrics.ToolSequences, currentSequence) - } - - metrics.TokenUsage = totalTokenUsage - metrics.Turns = turns - - // Convert tool call map to slice - for _, toolInfo := range toolCallMap { - metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo) - } - - // Sort tool calls by name for consistent output - sort.Slice(metrics.ToolCalls, func(i, j int) bool { - return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name - }) - - // Count errors and warnings using pattern matching for better accuracy - errorPatterns := e.GetErrorPatterns() - if len(errorPatterns) > 0 { - metrics.Errors = CountErrorsAndWarningsWithPatterns(logContent, errorPatterns) - } + // Finalize metrics using shared helper + FinalizeToolMetrics(&metrics, toolCallMap, currentSequence, turns, totalTokenUsage, logContent, e.GetErrorPatterns()) codexEngineLog.Printf("Parsed Codex metrics: turns=%d, token_usage=%d, tool_calls=%d, errors=%d", metrics.Turns, metrics.TokenUsage, len(metrics.ToolCalls), len(metrics.Errors)) diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index 2505db59d63..973fb8a69d4 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -489,29 +489,8 @@ func (e *CopilotEngine) ParseLogMetrics(logContent string, verbose bool) LogMetr // Basic processing - error/warning counting moved to end of function } - // Add final sequence if any - if len(currentSequence) > 0 { - metrics.ToolSequences = append(metrics.ToolSequences, currentSequence) - } - - metrics.TokenUsage = maxTokenUsage - metrics.Turns = turns - - // Convert tool call map to slice - for _, toolInfo := range toolCallMap { - metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo) - } - - // Sort tool calls by name for consistent output - sort.Slice(metrics.ToolCalls, func(i, j int) bool { - return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name - }) - - // Count errors and warnings using pattern matching for better accuracy - errorPatterns := e.GetErrorPatterns() - if len(errorPatterns) > 0 { - metrics.Errors = CountErrorsAndWarningsWithPatterns(logContent, errorPatterns) - } + // Finalize metrics using shared helper + FinalizeToolMetrics(&metrics, toolCallMap, currentSequence, turns, maxTokenUsage, logContent, e.GetErrorPatterns()) return metrics } diff --git a/pkg/workflow/metrics.go b/pkg/workflow/metrics.go index cca9e66ad16..7298aaf9bec 100644 --- a/pkg/workflow/metrics.go +++ b/pkg/workflow/metrics.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "regexp" + "sort" "strconv" "strings" "time" @@ -464,3 +465,64 @@ func extractErrorMessage(line string) string { return cleanedLine } + +// FinalizeToolMetrics completes the metric collection process by finalizing sequences, +// converting tool call maps to sorted slices, and optionally counting errors using patterns. +// This function is called by engine-specific ParseLogMetrics implementations to avoid code duplication. +func FinalizeToolMetrics( + metrics *LogMetrics, + toolCallMap map[string]*ToolCallInfo, + currentSequence []string, + turns int, + tokenUsage int, + logContent string, + errorPatterns []ErrorPattern, +) { + // Add final sequence if any + if len(currentSequence) > 0 { + metrics.ToolSequences = append(metrics.ToolSequences, currentSequence) + } + + metrics.TokenUsage = tokenUsage + metrics.Turns = turns + + // Convert tool call map to slice + for _, toolInfo := range toolCallMap { + metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo) + } + + // Sort tool calls by name for consistent output + sort.Slice(metrics.ToolCalls, func(i, j int) bool { + return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name + }) + + // Count errors and warnings using pattern matching for better accuracy + if len(errorPatterns) > 0 { + metrics.Errors = CountErrorsAndWarningsWithPatterns(logContent, errorPatterns) + } +} + +// FinalizeToolCallsAndSequence completes the tool call and sequence finalization. +// Use this function when the engine extracts token usage and turns from structured result entries, +// rather than accumulating them during line-by-line log parsing. This is a lighter version of +// FinalizeToolMetrics for engines that do not need to finalize token usage and turns here. +func FinalizeToolCallsAndSequence( + metrics *LogMetrics, + toolCallMap map[string]*ToolCallInfo, + currentSequence []string, +) { + // Add final sequence if any + if len(currentSequence) > 0 { + metrics.ToolSequences = append(metrics.ToolSequences, currentSequence) + } + + // Convert tool call map to slice + for _, toolInfo := range toolCallMap { + metrics.ToolCalls = append(metrics.ToolCalls, *toolInfo) + } + + // Sort tool calls by name for consistent output + sort.Slice(metrics.ToolCalls, func(i, j int) bool { + return metrics.ToolCalls[i].Name < metrics.ToolCalls[j].Name + }) +} diff --git a/pkg/workflow/metrics_test.go b/pkg/workflow/metrics_test.go index 800f37b5b7b..c31a5d9e353 100644 --- a/pkg/workflow/metrics_test.go +++ b/pkg/workflow/metrics_test.go @@ -704,3 +704,248 @@ func TestExtractErrorMessage(t *testing.T) { }) } } + +func TestFinalizeToolMetrics(t *testing.T) { + tests := []struct { + name string + initialMetrics LogMetrics + toolCallMap map[string]*ToolCallInfo + currentSequence []string + turns int + tokenUsage int + logContent string + errorPatterns []ErrorPattern + expectedTurns int + expectedTokens int + expectedToolLen int + expectedSeqLen int + expectedErrors int + }{ + { + name: "Basic finalization with sequence and tools", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{ + "bash": {Name: "bash", CallCount: 2}, + "github_search": {Name: "github_search", CallCount: 1}, + "web_fetch": {Name: "web_fetch", CallCount: 3}, + }, + currentSequence: []string{"bash", "github_search", "web_fetch"}, + turns: 5, + tokenUsage: 1500, + logContent: "", + errorPatterns: nil, + expectedTurns: 5, + expectedTokens: 1500, + expectedToolLen: 3, + expectedSeqLen: 1, + expectedErrors: 0, + }, + { + name: "Empty sequence should not be added", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{ + "bash": {Name: "bash", CallCount: 1}, + }, + currentSequence: []string{}, + turns: 2, + tokenUsage: 500, + logContent: "", + errorPatterns: nil, + expectedTurns: 2, + expectedTokens: 500, + expectedToolLen: 1, + expectedSeqLen: 0, + expectedErrors: 0, + }, + { + name: "Tools should be sorted by name", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{ + "zebra_tool": {Name: "zebra_tool", CallCount: 1}, + "alpha_tool": {Name: "alpha_tool", CallCount: 2}, + "middle_tool": {Name: "middle_tool", CallCount: 3}, + }, + currentSequence: []string{"zebra_tool", "alpha_tool"}, + turns: 3, + tokenUsage: 800, + logContent: "", + errorPatterns: nil, + expectedTurns: 3, + expectedTokens: 800, + expectedToolLen: 3, + expectedSeqLen: 1, + expectedErrors: 0, + }, + { + name: "Error patterns should be counted", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{}, + currentSequence: []string{}, + turns: 1, + tokenUsage: 100, + logContent: ` +Error: File not found +Warning: Deprecated API used +Error: Connection timeout +Info: Processing complete +`, + errorPatterns: []ErrorPattern{ + {Pattern: `(?i)error:?\s+(.+)`, LevelGroup: 0, MessageGroup: 1}, + {Pattern: `(?i)warning:?\s+(.+)`, LevelGroup: 0, MessageGroup: 1}, + }, + expectedTurns: 1, + expectedTokens: 100, + expectedToolLen: 0, + expectedSeqLen: 0, + expectedErrors: 3, // 2 errors + 1 warning + }, + { + name: "Existing sequences should be preserved", + initialMetrics: LogMetrics{ + ToolSequences: [][]string{ + {"tool1", "tool2"}, + }, + }, + toolCallMap: map[string]*ToolCallInfo{ + "tool3": {Name: "tool3", CallCount: 1}, + }, + currentSequence: []string{"tool3", "tool4"}, + turns: 2, + tokenUsage: 300, + logContent: "", + errorPatterns: nil, + expectedTurns: 2, + expectedTokens: 300, + expectedToolLen: 1, + expectedSeqLen: 2, // 1 existing + 1 new + expectedErrors: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + metrics := tt.initialMetrics + + FinalizeToolMetrics( + &metrics, + tt.toolCallMap, + tt.currentSequence, + tt.turns, + tt.tokenUsage, + tt.logContent, + tt.errorPatterns, + ) + + if metrics.Turns != tt.expectedTurns { + t.Errorf("Expected %d turns, got %d", tt.expectedTurns, metrics.Turns) + } + + if metrics.TokenUsage != tt.expectedTokens { + t.Errorf("Expected %d tokens, got %d", tt.expectedTokens, metrics.TokenUsage) + } + + if len(metrics.ToolCalls) != tt.expectedToolLen { + t.Errorf("Expected %d tool calls, got %d", tt.expectedToolLen, len(metrics.ToolCalls)) + } + + if len(metrics.ToolSequences) != tt.expectedSeqLen { + t.Errorf("Expected %d sequences, got %d", tt.expectedSeqLen, len(metrics.ToolSequences)) + } + + // Verify tools are sorted by name + if len(metrics.ToolCalls) > 1 { + for i := 0; i < len(metrics.ToolCalls)-1; i++ { + if metrics.ToolCalls[i].Name > metrics.ToolCalls[i+1].Name { + t.Errorf("Tool calls not sorted: %s comes before %s", + metrics.ToolCalls[i].Name, metrics.ToolCalls[i+1].Name) + } + } + } + + if len(metrics.Errors) != tt.expectedErrors { + t.Errorf("Expected %d errors/warnings, got %d", tt.expectedErrors, len(metrics.Errors)) + } + }) + } +} + +func TestFinalizeToolCallsAndSequence(t *testing.T) { + tests := []struct { + name string + initialMetrics LogMetrics + toolCallMap map[string]*ToolCallInfo + currentSequence []string + expectedToolLen int + expectedSeqLen int + }{ + { + name: "Basic finalization with tools and sequence", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{ + "bash": {Name: "bash", CallCount: 2}, + "github_search": {Name: "github_search", CallCount: 1}, + }, + currentSequence: []string{"bash", "github_search"}, + expectedToolLen: 2, + expectedSeqLen: 1, + }, + { + name: "Empty sequence should not be added", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{"bash": {Name: "bash", CallCount: 1}}, + currentSequence: []string{}, + expectedToolLen: 1, + expectedSeqLen: 0, + }, + { + name: "Tools should be sorted alphabetically", + initialMetrics: LogMetrics{}, + toolCallMap: map[string]*ToolCallInfo{ + "zebra": {Name: "zebra", CallCount: 1}, + "alpha": {Name: "alpha", CallCount: 2}, + "middle": {Name: "middle", CallCount: 3}, + }, + currentSequence: []string{"zebra", "alpha"}, + expectedToolLen: 3, + expectedSeqLen: 1, + }, + { + name: "Preserves existing sequences", + initialMetrics: LogMetrics{ + ToolSequences: [][]string{ + {"tool1", "tool2"}, + }, + }, + toolCallMap: map[string]*ToolCallInfo{"tool3": {Name: "tool3", CallCount: 1}}, + currentSequence: []string{"tool3"}, + expectedToolLen: 1, + expectedSeqLen: 2, // 1 existing + 1 new + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + metrics := tt.initialMetrics + + FinalizeToolCallsAndSequence(&metrics, tt.toolCallMap, tt.currentSequence) + + if len(metrics.ToolCalls) != tt.expectedToolLen { + t.Errorf("Expected %d tool calls, got %d", tt.expectedToolLen, len(metrics.ToolCalls)) + } + + if len(metrics.ToolSequences) != tt.expectedSeqLen { + t.Errorf("Expected %d sequences, got %d", tt.expectedSeqLen, len(metrics.ToolSequences)) + } + + // Verify tools are sorted by name + if len(metrics.ToolCalls) > 1 { + for i := 0; i < len(metrics.ToolCalls)-1; i++ { + if metrics.ToolCalls[i].Name > metrics.ToolCalls[i+1].Name { + t.Errorf("Tool calls not sorted: %s comes before %s", + metrics.ToolCalls[i].Name, metrics.ToolCalls[i+1].Name) + } + } + } + }) + } +}