Skip to content

[fp-enhancer] Improve pkg/cli: replace bubble sort, use sliceutil.Map, migrate to slices package#20323

Merged
pelikhan merged 1 commit intomainfrom
fp-enhancer/pkg-cli-improvements-bf9afd5b9f317c9e
Mar 10, 2026
Merged

[fp-enhancer] Improve pkg/cli: replace bubble sort, use sliceutil.Map, migrate to slices package#20323
pelikhan merged 1 commit intomainfrom
fp-enhancer/pkg-cli-improvements-bf9afd5b9f317c9e

Conversation

@github-actions
Copy link
Contributor

Functional and immutability improvements to the pkg/cli package. This is run 1 of a systematic round-robin series covering all packages.

Next package to process: pkg/console


Summary

Three files updated with tasteful functional improvements:

File Change Impact
health_command.go Bubble sort → slices.SortFunc O(n²)→O(n log n), 8 lines → 3 lines
compile_config.go Nested loops → sliceutil.Map + pure helper Declarative, composable
logs_report.go sort.Slice/sort.Stringsslices.SortFunc/slices.Sort Type-safe, index-free, removes sort import

Net: 38 insertions, 53 deletions (net −15 lines)


1. Replace Bubble Sort with slices.SortFunc (health_command.go)

Before — O(n²) bubble sort with a comment apologizing for it:

// 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]
        }
    }
}

After — O(n log n), expressive, uses the standard slices package:

// 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)
})

2. Declarative Sanitization with sliceutil.Map (compile_config.go)

Before — nested imperative loops with index tracking:

sanitized := make([]ValidationResult, len(results))
for i, result := range results {
    sanitized[i] = ValidationResult{
        ...
        Errors:   make([]CompileValidationError, len(result.Errors)),
        Warnings: make([]CompileValidationError, len(result.Warnings)),
    }
    for j, err := range result.Errors {
        sanitized[i].Errors[j] = CompileValidationError{...}
    }
    for j, warn := range result.Warnings {
        sanitized[i].Warnings[j] = CompileValidationError{...}
    }
}
return sanitized

After — extracted pure helper + declarative nested Map:

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:       sliceutil.Map(result.Errors, sanitizeError),
        Warnings:     sliceutil.Map(result.Warnings, sanitizeError),
    }
})

3. Type-safe Sorting with slices.SortFunc (logs_report.go)

Replaced 6 sort.Slice + 3 sort.Strings calls with their type-safe equivalents from the slices package. The sort package import is removed entirely.

Benefits:

  • Comparison functions receive typed values (a, b T) instead of index-based result[i]/result[j]
  • cmp.Compare eliminates hand-written comparison logic
  • slices.Sort replaces sort.Strings consistently
  • Aligns with existing use of slices.Contains already in the file

Principles Applied

  1. Pure function extraction: sanitizeError is a pure function — same input always gives same output, no side effects, easily testable in isolation
  2. Declarative over imperative: sliceutil.Map expresses what to transform, not how to iterate
  3. Standard library preference: slices package (Go 1.21+) over sort package for type-safe slice operations
  4. No behavioral changes: All transformations are functionally equivalent

Testing

  • TestHealthConfigValidation, TestHealthCommand, TestCalculateWorkflowHealth, TestGroupRunsByWorkflow, TestCalculateHealthSummary — all pass
  • TestBuildMCPToolUsageSummarySorting — passes (validates sort order maintained)
  • go build ./pkg/cli/ — clean
  • go vet ./pkg/cli/ — clean
  • make fmt — no changes needed

Generated by Functional Pragmatist ·

  • expires on Mar 11, 2026, 9:31 AM UTC

- 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>
@pelikhan pelikhan marked this pull request as ready for review March 10, 2026 10:57
Copilot AI review requested due to automatic review settings March 10, 2026 10:57
@pelikhan pelikhan merged commit 297b343 into main Mar 10, 2026
@pelikhan pelikhan deleted the fp-enhancer/pkg-cli-improvements-bf9afd5b9f317c9e branch March 10, 2026 10:57
Copy link
Contributor

Copilot AI left a comment

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 modernizes pkg/cli by replacing index-based sorting and imperative slice transformations with Go’s slices/cmp helpers and a small functional mapping utility, improving readability and reducing algorithmic complexity where applicable.

Changes:

  • Replaced a manual bubble sort with slices.SortFunc + cmp.Compare in health summary sorting.
  • Refactored validation result sanitization to a pure helper + nested sliceutil.Map transforms.
  • Migrated multiple sort.Slice/sort.Strings usages to slices.SortFunc/slices.Sort, removing the sort import.

Reviewed changes

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

File Description
pkg/cli/health_command.go Replaces bubble sort with slices.SortFunc for success-rate ordering.
pkg/cli/compile_config.go Uses sliceutil.Map and a pure sanitizer helper to produce sanitized validation output.
pkg/cli/logs_report.go Replaces index-based sorting with type-safe slices sorting and cmp.Compare.

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

You can also share your feedback on Copilot code review. Take the survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants