Skip to content

[refactor] Semantic Function Clustering Analysis — Dead Code, Duplicate Logic, and Thin Wrappers #24467

Description

@github-actions

This report covers the semantic function clustering analysis of 658 non-test Go source files across 20 packages in pkg/. The analysis used Serena LSP tooling, direct grep, and cross-package deduplication checks.

Executive Summary

Metric Count
Go files analyzed 658
Packages scanned 20
Issues identified 6
Dead code packages 1
Duplicate function implementations 2
Deprecated functions still in use 1
Thin wrappers adding no value 3

Issue 1 — Dead Code: pkg/mathutil Package (High Priority)

File: pkg/mathutil/mathutil.go

The entire mathutil package is dead code. It defines two functions:

func Min(a, b int) int { ... }
func Max(a, b int) int { ... }

Neither function is imported or called anywhere in the codebase (0 callers, confirmed by exhaustive grep). The project runs on Go 1.25.8, which includes built-in min/max functions (added in Go 1.21), making this package doubly redundant.

Evidence: grep -r "mathutil" /home/runner/work/gh-aw/gh-aw --include="*.go" | grep -v "_test.go" | grep -v "/pkg/mathutil/" returns 0 results.

Recommendation: Delete pkg/mathutil/mathutil.go and pkg/mathutil/mathutil_test.go entirely.

Estimated Impact: Removes 2 files, ~35 lines. No callers to update.


Issue 2 — Duplicate Deduplication Logic (Medium Priority)

File: pkg/workflow/strict_mode_steps_validation.go

The function deduplicateStringSlice is defined locally:

// strict_mode_steps_validation.go
func deduplicateStringSlice(in []string) []string {
    seen := make(map[string]bool, len(in))
    out := make([]string, 0, len(in))
    for _, s := range in {
        if !seen[s] {
            seen[s] = true
            out = append(out, s)
        }
    }
    return out
}

This is functionally identical to the existing generic utility sliceutil.Deduplicate[T comparable]:

// pkg/sliceutil/sliceutil.go
func Deduplicate[T comparable](slice []T) []T {
    seen := make(map[T]bool, len(slice))
    result := make([]T, 0, len(slice))
    for _, item := range slice {
        if !seen[item] {
            seen[item] = true
            result = append(result, item)
        }
    }
    return result
}

The local function is used in exactly 1 place (same file).

Recommendation: Replace deduplicateStringSlice(secretRefs) with sliceutil.Deduplicate(secretRefs) and delete the private function.

Estimated Impact: Removes ~12 lines, improves consistency with the sliceutil package.


Issue 3 — Deprecated Function Still In Active Use (Low-Medium Priority)

Files: pkg/cli/token_usage.go, pkg/cli/audit_report_render.go

cli.FormatDurationMs is already marked deprecated in the source:

// pkg/cli/token_usage.go:267
// Deprecated: Use timeutil.FormatDurationMs instead.
func FormatDurationMs(ms int) string {
    return timeutil.FormatDurationMs(ms)
}

Yet it still has 2 active call sites in the same package:

  • pkg/cli/token_usage.go:290AvgDuration: FormatDurationMs(avgDur)
  • pkg/cli/audit_report_render.goFormatDurationMs(summary.AvgDurationMs())

Recommendation: Replace both call sites with direct timeutil.FormatDurationMs(...) calls, then delete the deprecated wrapper function.

Estimated Impact: Removes ~5 lines, eliminates a function the codebase itself says should not exist.


Issue 4 — Thin Wrappers Over stdlib/utility Functions (Low Priority)

4a. sliceutil.Contains wraps slices.Contains

File: pkg/sliceutil/sliceutil.go

// sliceutil.go
func Contains(slice []string, item string) bool {
    return slices.Contains(slice, item)
}

This is a type-restricted wrapper over slices.Contains from the Go standard library with no added behavior. It has 7 callers across the codebase. The standard library function is already generic and accepts []string directly.

4b. formatDurationNs wraps timeutil.FormatDurationNs

File: pkg/cli/audit_cross_run_render.go

// audit_cross_run_render.go:390
func formatDurationNs(ns int64) string {
    return timeutil.FormatDurationNs(ns)
}

Used in 6 call sites within the same file. A one-line function for a one-line implementation adds no semantic value.

Recommendation: For 4a, callers can be updated to use slices.Contains directly (import "slices"). For 4b, inline timeutil.FormatDurationNs at the 6 call sites and delete the wrapper. These are low-risk mechanical changes.


Issue 5 — Near-Duplicate Sanitize Functions (Low Priority)

File: pkg/stringutil/sanitize.go

SanitizeParameterName (for JavaScript) and SanitizePythonVariableName (for Python) are nearly identical:

// JavaScript version — allows a-z, A-Z, 0-9, _, $
func SanitizeParameterName(name string) string {
    result := strings.Map(func(r rune) rune {
        if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' || r == '$' {
            return r
        }
        return '_'
    }, name)
    if len(result) > 0 && result[0] >= '0' && result[0] <= '9' {
        result = "_" + result
    }
    return result
}

// Python version — allows a-z, A-Z, 0-9, _ (no $)
func SanitizePythonVariableName(name string) string {
    result := strings.Map(func(r rune) rune {
        if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' {
            return r
        }
        return '_'
    }, name)
    if len(result) > 0 && result[0] >= '0' && result[0] <= '9' {
        result = "_" + result
    }
    return result
}

The only difference is that $ is a valid JavaScript identifier character but not a Python one. The two functions share ~95% of their implementation.

Recommendation: Extract a shared sanitizeIdentifierName(name string, extraAllowed func(rune) bool) string helper called by both public functions. This is a minor refactoring but reduces duplication in a security-sensitive area.


Issue 6 — Misplaced Functions in compiler_yaml_helpers.go (Low Priority)

File: pkg/workflow/compiler_yaml_helpers.go

This file is named as a YAML helper, but contains two functions unrelated to YAML:

  • ContainsCheckout(customSteps string) bool — checks if custom step strings include actions/checkout
  • GetWorkflowIDFromPath(markdownPath string) string — extracts a workflow ID from a .md file path

These are general workflow utility functions, not YAML-specific. They are called from multiple files (compiler_orchestrator_workflow.go, compiler_string_api.go, compiler_safe_outputs_job.go, compiler_yaml_main_job.go, compiler_jobs.go).

The file also has trailing comments pointing to implementations elsewhere:

// generateGitHubScriptWithRequire is implemented in compiler_github_actions_steps.go
// generateInlineGitHubScriptStep is implemented in compiler_github_actions_steps.go

Recommendation: Move ContainsCheckout and GetWorkflowIDFromPath to compiler.go or a new compiler_workflow_helpers.go file. Remove the dangling comments referencing functions in other files.


Function Cluster Summary

Validation File Cluster (56 files — well-organized)

The workflow package has 56 validation files following a consistent *_validation.go naming pattern. This is well-organized — each file handles a distinct concern (cache, concurrency, firewall, secrets, strict-mode, etc.). No outliers found.

Engine Cluster (Claude / Codex / Copilot — interface-based, expected)

claude_engine.go, codex_engine.go, and copilot_engine.go share method signatures (e.g., GetRequiredSecretNames, GetInstallationSteps, GetFirewallLogsCollectionStep) because they implement a common CodingAgentEngine interface. This is intentional polymorphism, not duplication.

Update Entity Cluster (well-designed)

update_issue_helpers.go, update_discussion_helpers.go, update_pull_request_helpers.go follow a consistent pattern, all delegating to a shared update_entity_helpers.go generic implementation. Well-structured.

Utility Package Overview
Package Files Primary Functions Notes
mathutil 1 Min, Max Dead code — 0 callers
sliceutil 1 Contains, Filter, Map, Deduplicate, etc. Contains is a trivial stdlib wrapper
stringutil 6 Sanitize, normalize, URL, ANSI, PAT validation Minor duplication in sanitize functions
timeutil 1 FormatDuration* Partially shadowed by deprecated cli wrapper
semverutil 1 Compare, IsCompatible, ParseVersion Re-wrapped by workflow/semver.go

Refactoring Recommendations by Priority

Priority 1: High Impact, Low Risk

  1. Delete pkg/mathutil — 0 callers, dead code, superseded by Go builtins. Estimated effort: 5 minutes.
  2. Remove deduplicateStringSlice in strict_mode_steps_validation.go and use sliceutil.Deduplicate[string]. Estimated effort: 10 minutes.
  3. Complete the deprecation of cli.FormatDurationMs — 2 remaining call sites. Estimated effort: 10 minutes.

Priority 2: Medium Impact, Low Risk

  1. Remove formatDurationNs wrapper in audit_cross_run_render.go — inline timeutil.FormatDurationNs. Estimated effort: 15 minutes.
  2. Relocate ContainsCheckout and GetWorkflowIDFromPath from compiler_yaml_helpers.go to a more appropriately named file. Estimated effort: 30 minutes.

Priority 3: Low Impact, Moderate Effort

  1. Unify SanitizeParameterName / SanitizePythonVariableName with a shared internal helper. Estimated effort: 1 hour.
  2. Replace sliceutil.Contains usages (7 sites) with direct slices.Contains calls. Estimated effort: 30 minutes.

Implementation Checklist

  • Delete pkg/mathutil/mathutil.go and pkg/mathutil/mathutil_test.go
  • Replace deduplicateStringSlice with sliceutil.Deduplicate[string] in strict_mode_steps_validation.go
  • Update 2 call sites of cli.FormatDurationMs to use timeutil.FormatDurationMs
  • Delete deprecated cli.FormatDurationMs wrapper
  • Inline timeutil.FormatDurationNs at 6 call sites in audit_cross_run_render.go
  • Move ContainsCheckout and GetWorkflowIDFromPath to appropriate files
  • Consider unifying SanitizeParameterName / SanitizePythonVariableName
  • Consider inlining sliceutil.Contains usages to use stdlib slices.Contains

Analysis Metadata

  • Total Go Files Analyzed: 658
  • Packages Scanned: 20 (workflow, cli, parser, console, stringutil, sliceutil, mathutil, timeutil, semverutil, repoutil, gitutil, envutil, fileutil, agentdrain, constants, logger, styles, tty, testutil, types)
  • Function Clusters Identified: 6 major clusters (validation, engine, entity-update, utility, sanitize, format)
  • Dead Code Found: 1 package (mathutil)
  • Duplicate Implementations: 2 (deduplicateStringSlice, FormatDurationMs wrapper)
  • Thin Wrappers: 3 (sliceutil.Contains, formatDurationNs, cli.FormatDurationMs)
  • Misplaced Functions: 2 (ContainsCheckout, GetWorkflowIDFromPath in compiler_yaml_helpers.go)
  • Detection Method: Serena LSP semantic analysis + grep cross-reference + diff-based pattern matching
  • Analysis Run: §23977882320

References:

Generated by Semantic Function Refactoring · ● 425K ·

  • expires on Apr 6, 2026, 11:31 AM UTC

Activity

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

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions