[refactor] Semantic Function Clustering Analysis — pkg/ refactoring opportunities

[github-actions]

Analysis of 687 non-test Go files across 20 packages in pkg/. No existing [refactor] issues were found before this run.

Summary

Metric	Value
Files analyzed	687
Packages surveyed	20
Outlier functions identified	2
Near-duplicate clusters detected	1 (4 functions)
Thin wrapper candidates	1
Overall codebase organization	Good — one-file-per-feature is consistently applied

The codebase is generally well-organized. The compiler_*.go / safe_outputs_*.go / update_*_helpers.go families are coherent and follow clear patterns. The findings below are medium-to-low impact improvements.

---

Issue 1 — Near-duplicate any → []string conversion (4 variants, HIGH impact)

Problem: Four functions in pkg/workflow all implement the same core type-switch logic to convert an untyped any value into []string. They differ only in error-handling style and call site, making it unclear which to use.

Function	File	Strategy
toStringSlice	compiler_filters_validation.go:209	Returns ([]string, error) — rejects non-strings
parseStringSliceAny	validation_helpers.go:166	Returns []string, skips non-strings, logs with *logger.Logger
extractStringSliceField	role_checks.go:510	Returns []string, skips non-strings, logs with package-level roleLog
extractStringSliceFromConfig	safe_outputs_config_helpers.go:215	Thin wrapper around parseStringSliceAny that adds a map-key lookup

View code comparison

// compiler_filters_validation.go:209 — returns error on bad item
func toStringSlice(val any) ([]string, error) {
    switch v := val.(type) {
    case []string:
        return v, nil
    case []any:
        for _, item := range v {
            s, ok := item.(string)
            if !ok {
                return nil, errors.New("non-string item in list")
            }
            ...
        }
    case string:
        return []string{v}, nil
    default:
        return nil, fmt.Errorf("unsupported type %T", val)
    }
}

// validation_helpers.go:166 — skips bad items, logs via *logger.Logger
func parseStringSliceAny(raw any, log *logger.Logger) []string {
    switch v := raw.(type) {
    case []string:
        return v
    case []any:
        for _, item := range v {
            if s, ok := item.(string); ok {
                result = append(result, s)
            } else if log != nil {
                log.Printf("skipping non-string item: %T", item)
            }
        }
    ...
    }
}

// role_checks.go:510 — same pattern, different logger
func extractStringSliceField(value any, fieldName string) []string { ... }

// safe_outputs_config_helpers.go:215 — thin wrapper
func extractStringSliceFromConfig(config map[string]any, key string) []string {
    ...
    return parseStringSliceAny(raw, nil)
}

Recommendation: Consolidate toStringSlice and extractStringSliceField into validation_helpers.go, using parseStringSliceAny as the canonical implementation. The only meaningful distinction is the error-returning variant (toStringSlice); this could be a parameter or a separate exported function if the strict-reject behavior is intentional.

• Move toStringSlice → validation_helpers.go (or make it call parseStringSliceAny and validate separately)
• Move extractStringSliceField → validation_helpers.go (or make it call parseStringSliceAny)
• Keep extractStringSliceFromConfig as a map-lookup helper that delegates to parseStringSliceAny

Estimated effort: 1–2 hours | Benefit: Single source of truth for any → []string conversion logic

---

Issue 2 — normalizeHeredocDelimiters outlier in compiler.go (MEDIUM impact)

Problem: normalizeHeredocDelimiters is a string-processing function defined in compiler.go:30, but all other heredoc-related functions live in strings.go.

Function	Current file	Correct file
normalizeHeredocDelimiters	compiler.go:30	strings.go
GenerateHeredocDelimiterFromSeed	strings.go:280	✅
ValidateHeredocContent	strings.go:312	✅
ValidateHeredocDelimiter	strings.go:329	✅

strings.go already has a well-documented section on heredoc utilities. normalizeHeredocDelimiters directly depends on the same GH_AW_* delimiter format and fits that section perfectly.

Recommendation: Move normalizeHeredocDelimiters (and its heredocDelimiterRE regex variable) from compiler.go to strings.go.

Estimated effort: 30 minutes | Benefit: All heredoc string logic in one place

---

Issue 3 — toStringSlice placement outlier (MEDIUM impact)

Problem: toStringSlice is a general type-conversion utility that lives in compiler_filters_validation.go — a file dedicated to GitHub Actions event filter validation. The function has no semantic relation to filter/event logic.

File purpose: compiler_filters_validation.go validates event filters like push, pull_request, and glob patterns.
Function purpose: toStringSlice converts any any value to []string — a general utility.

Recommendation: Move toStringSlice to validation_helpers.go (which already documents itself as the home for shared validation utilities) or merge it with parseStringSliceAny as described in Issue 1.

Estimated effort: 15 minutes | Benefit: Clearer file responsibilities

---

Issue 4 — ParseBoolFromConfig thin wrapper (LOW impact)

Problem: ParseBoolFromConfig in config_helpers.go:185 adds only debug logging around typeutil.ParseBool. The implementation is:

func ParseBoolFromConfig(m map[string]any, key string, log *logger.Logger) bool {
    if log != nil { log.Printf("Parsing %s from config", key) }
    result := typeutil.ParseBool(m, key)
    if log != nil { log.Printf("Parsed %s from config: %t", key, result) }
    return result
}

This wrapper exists in 14 call sites. The logging it adds is low-signal (parse-level debug output). Call sites could instead call typeutil.ParseBool directly when logging is not needed, or the wrapper's value could be made explicit in its doc comment.

Recommendation: Either document the wrapper's purpose clearly (logging convention for config parsing), or evaluate whether call sites need the extra logging. No immediate action required — this is a smell to watch rather than refactor now.

---

What is NOT an issue

• update_entity_helpers.go + update_issue_helpers.go + update_discussion_helpers.go + update_pull_request_helpers.go: This is well-organized. Entity-specific thin wrappers delegate to a generic parseUpdateEntityConfig, and generics (parseUpdateEntityConfigTyped[T any]) are already used.
• map_helpers.go: The map_helpers.go comment explicitly references typeutil for type conversion and positions excludeMapKeys / sortedMapKeys as generic map utilities — correctly documented.
• stringutil/ vs workflow/strings.go: The workflow/strings.go file's comment explicitly explains the distinction from stringutil (domain-specific sanitize patterns vs general-purpose utilities). This is intentional and documented.
• sliceutil.Deduplicate vs mergeStringSlicesDedup: mergeStringSlicesDedup correctly delegates to sliceutil.Deduplicate — no duplication.

---

Implementation Checklist

• Issue 1 (HIGH): Consolidate 4 near-duplicate any → []string functions into validation_helpers.go
  • Move/merge toStringSlice → use or call parseStringSliceAny
  • Move extractStringSliceField → use parseStringSliceAny
  • Update all call sites
  • Verify tests pass
• Issue 2 (MEDIUM): Move normalizeHeredocDelimiters + heredocDelimiterRE from compiler.go to strings.go
• Issue 3 (MEDIUM): Move toStringSlice from compiler_filters_validation.go to validation_helpers.go
• Issue 4 (LOW): Review ParseBoolFromConfig call sites for logging necessity

---

Analysis Metadata

• Total Go files analyzed: 687 (non-test, pkg/ only)
• Packages surveyed: 20 (agentdrain, cli, console, constants, envutil, fileutil, gitutil, logger, parser, repoutil, semverutil, sliceutil, stringutil, styles, testutil, timeutil, tty, types, typeutil, workflow)
• Detection method: Serena LSP semantic analysis + grep-based naming pattern analysis
• Workflow run: §24452248920
• Analysis date: 2026-04-15

Generated by Semantic Function Refactoring · ● 268.3K · ◷

• expires on Apr 17, 2026, 11:41 AM UTC