From 337d6e01e48615b915bc5b9174bb75cc68578ab0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 10 Mar 2026 09:28:11 +0000 Subject: [PATCH] refactor(cli): apply functional/immutability improvements to pkg/cli MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - health_command.go: Replace O(n²) bubble sort with slices.SortFunc for workflow health sorting (cleaner, more efficient, uses standard library) - compile_config.go: Use sliceutil.Map for nested validation result sanitization, extracting a pure sanitizeError helper function to make the transformation declarative and composable - logs_report.go: Replace all sort.Slice/sort.Strings calls with slices.SortFunc/slices.Sort from the standard slices package for type-safe, index-free comparisons; removes sort package dependency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cli/compile_config.go | 40 ++++++++++++++------------------------- pkg/cli/health_command.go | 15 ++++++--------- pkg/cli/logs_report.go | 36 +++++++++++++++++------------------ 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/pkg/cli/compile_config.go b/pkg/cli/compile_config.go index 9fa358deba9..15888ae4283 100644 --- a/pkg/cli/compile_config.go +++ b/pkg/cli/compile_config.go @@ -2,6 +2,7 @@ package cli import ( "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/sliceutil" "github.com/github/gh-aw/pkg/stringutil" ) @@ -78,34 +79,21 @@ func sanitizeValidationResults(results []ValidationResult) []ValidationResult { compileConfigLog.Printf("Sanitizing validation results: workflow_count=%d", len(results)) - sanitized := make([]ValidationResult, len(results)) - for i, result := range results { - sanitized[i] = ValidationResult{ + sanitizeError := func(e CompileValidationError) CompileValidationError { + return CompileValidationError{ + Type: e.Type, + Message: stringutil.SanitizeErrorMessage(e.Message), + Line: e.Line, + } + } + + return sliceutil.Map(results, func(result ValidationResult) ValidationResult { + return ValidationResult{ Workflow: result.Workflow, Valid: result.Valid, CompiledFile: result.CompiledFile, - Errors: make([]CompileValidationError, len(result.Errors)), - Warnings: make([]CompileValidationError, len(result.Warnings)), - } - - // Sanitize all error messages - for j, err := range result.Errors { - sanitized[i].Errors[j] = CompileValidationError{ - Type: err.Type, - Message: stringutil.SanitizeErrorMessage(err.Message), - Line: err.Line, - } + Errors: sliceutil.Map(result.Errors, sanitizeError), + Warnings: sliceutil.Map(result.Warnings, sanitizeError), } - - // Sanitize all warning messages - for j, warn := range result.Warnings { - sanitized[i].Warnings[j] = CompileValidationError{ - Type: warn.Type, - Message: stringutil.SanitizeErrorMessage(warn.Message), - Line: warn.Line, - } - } - } - - return sanitized + }) } diff --git a/pkg/cli/health_command.go b/pkg/cli/health_command.go index 4085ffedda1..97b70d50afa 100644 --- a/pkg/cli/health_command.go +++ b/pkg/cli/health_command.go @@ -1,9 +1,11 @@ package cli import ( + "cmp" "encoding/json" "fmt" "os" + "slices" "strconv" "strings" "time" @@ -202,15 +204,10 @@ func displayHealthSummary(runs []WorkflowRun, config HealthConfig) error { workflowHealths = append(workflowHealths, health) } - // Sort by success rate (lowest first to highlight issues) - // Using a simple bubble sort for simplicity - for i := 0; i < len(workflowHealths); i++ { - for j := i + 1; j < len(workflowHealths); j++ { - if workflowHealths[i].SuccessRate > workflowHealths[j].SuccessRate { - workflowHealths[i], workflowHealths[j] = workflowHealths[j], workflowHealths[i] - } - } - } + // Sort by success rate ascending (lowest first to highlight issues) + slices.SortFunc(workflowHealths, func(a, b WorkflowHealth) int { + return cmp.Compare(a.SuccessRate, b.SuccessRate) + }) // Calculate summary summary := CalculateHealthSummary(workflowHealths, fmt.Sprintf("Last %d Days", config.Days), config.Threshold) diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index 0fd1dd251f9..8df933ddd40 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -1,12 +1,12 @@ package cli import ( + "cmp" "encoding/json" "fmt" "os" "path/filepath" "slices" - "sort" "strings" "time" @@ -368,8 +368,8 @@ func buildToolUsageSummary(processedRuns []ProcessedRun) []ToolUsageSummary { } // Sort by total calls descending - sort.Slice(result, func(i, j int) bool { - return result[i].TotalCalls > result[j].TotalCalls + slices.SortFunc(result, func(a, b ToolUsageSummary) int { + return cmp.Compare(b.TotalCalls, a.TotalCalls) }) return result @@ -453,8 +453,8 @@ func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary ) // Sort by count descending - sort.Slice(result, func(i, j int) bool { - return result[i].Count > result[j].Count + slices.SortFunc(result, func(a, b MissingToolSummary) int { + return cmp.Compare(b.Count, a.Count) }) return result @@ -496,8 +496,8 @@ func buildMissingDataSummary(processedRuns []ProcessedRun) []MissingDataSummary ) // Sort by count descending - sort.Slice(result, func(i, j int) bool { - return result[i].Count > result[j].Count + slices.SortFunc(result, func(a, b MissingDataSummary) int { + return cmp.Compare(b.Count, a.Count) }) return result @@ -537,8 +537,8 @@ func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary { ) // Sort by count descending - sort.Slice(result, func(i, j int) bool { - return result[i].Count > result[j].Count + slices.SortFunc(result, func(a, b MCPFailureSummary) int { + return cmp.Compare(b.Count, a.Count) }) return result @@ -587,12 +587,12 @@ func convertDomainsToSortedSlices(allowedMap, blockedMap map[string]bool) (allow for domain := range allowedMap { allowed = append(allowed, domain) } - sort.Strings(allowed) + slices.Sort(allowed) for domain := range blockedMap { blocked = append(blocked, domain) } - sort.Strings(blocked) + slices.Sort(blocked) return allowed, blocked } @@ -704,7 +704,7 @@ func buildRedactedDomainsSummary(processedRuns []ProcessedRun) *RedactedDomainsL for domain := range allDomainsSet { allDomains = append(allDomains, domain) } - sort.Strings(allDomains) + slices.Sort(allDomains) return &RedactedDomainsLogSummary{ TotalDomains: len(allDomains), @@ -825,16 +825,16 @@ func buildMCPToolUsageSummary(processedRuns []ProcessedRun) *MCPToolUsageSummary } // Sort summaries by server name, then tool name - sort.Slice(summaries, func(i, j int) bool { - if summaries[i].ServerName != summaries[j].ServerName { - return summaries[i].ServerName < summaries[j].ServerName + slices.SortFunc(summaries, func(a, b MCPToolSummary) int { + if a.ServerName != b.ServerName { + return cmp.Compare(a.ServerName, b.ServerName) } - return summaries[i].ToolName < summaries[j].ToolName + return cmp.Compare(a.ToolName, b.ToolName) }) // Sort servers by name - sort.Slice(servers, func(i, j int) bool { - return servers[i].ServerName < servers[j].ServerName + slices.SortFunc(servers, func(a, b MCPServerStats) int { + return cmp.Compare(a.ServerName, b.ServerName) }) reportLog.Printf("Built MCP tool usage summary: %d tool summaries, %d servers, %d total tool calls",