Skip to content

[refactor] Semantic Function Clustering Analysis #6023

@github-actions

Description

@github-actions

Overview

An automated semantic function clustering analysis was performed on all 126 non-test Go source files in internal/, cataloging 807 top-level functions across 23 packages. The codebase is well-organized overall with clear package boundaries and consistent naming conventions. Three actionable refactoring opportunities were identified — one of which is a new finding (Docker -e flag iteration duplication) not present in prior analyses.


Analysis Metadata

  • Total Go files analyzed: 126 (excluding *_test.go)
  • Total top-level functions cataloged: 807
  • Function clusters identified: ~23 (by package/prefix)
  • Outliers found: 1 (JSON extraction helpers in DIFC log file)
  • Duplicates / near-duplicates detected: 2
  • Detection method: Naming-pattern analysis + cross-package function comparison

Identified Issues

1. 🆕 Docker -e Flag Iteration Logic Duplicated Between launcher and envutil

Files:

  • internal/launcher/log_helpers.gologEnvPassthrough(args []string)
  • internal/envutil/expand_env_args.goExpandEnvArgs(args []string) []string

Both functions contain an identical loop skeleton that scans a Docker args slice for -e VAR pairs (where the value has no =):

// launcher/log_helpers.go
for i := 0; i < len(args); i++ {
    arg := args[i]
    if arg == "-e" && i+1 < len(args) {
        nextArg := args[i+1]
        if !strings.Contains(nextArg, "=") {
            if val := os.Getenv(nextArg); val != "" {
                log.Printf("[LAUNCHER] ✓ Env passthrough: %s=%s ...", nextArg, sanitize.TruncateSecret(val))
            } else {
                log.Printf("[LAUNCHER] ✗ WARNING: Env passthrough for %s NOT FOUND ...", nextArg)
            }
        }
        i++ // Skip next arg
    }
}

// envutil/expand_env_args.go
for i := 0; i < len(args); i++ {
    arg := args[i]
    if arg == "-e" && i+1 < len(args) {
        nextArg := args[i+1]
        if len(nextArg) > 0 && !strings.Contains(nextArg, "=") {
            if value, exists := os.LookupEnv(nextArg); exists {
                result = append(result, "-e", fmt.Sprintf("%s=%s", nextArg, value))
                i++ // Skip next arg
                continue
            }
        }
    }
    result = append(result, arg)
}

The detection logic (arg == "-e", !strings.Contains(nextArg, "="), i++ skip) is structurally identical. If the Docker args format ever changes (e.g., --env support), both sites would need updating.

Recommendation: Extract a shared helper in envutil — e.g.:

// envutil/expand_env_args.go
// WalkDockerEnvArgs iterates args calling fn(varName, value, found) for each -e VAR pair.
func WalkDockerEnvArgs(args []string, fn func(varName, value string, found bool)) []string { ... }

Then logEnvPassthrough and ExpandEnvArgs both delegate the walk to WalkDockerEnvArgs.

Estimated effort: 1–2 hours
Risk: Low — pure refactor with no behavior change


2. Generic JSON Map Extraction Helpers Buried in server/difc_log.go

File: internal/server/difc_log.go

Three generic map[string]interface{} extraction utilities live alongside DIFC-specific filtering log helpers:

// Generic enough to belong in a shared util package:
func getFilteredItemStringField(m map[string]interface{}, fields ...string) string { ... }
func extractAuthorLogin(m map[string]interface{}) string { ... }
func extractNumberField(m map[string]interface{}) string { ... }

These are currently only used in DIFC log formatting, but the patterns they encode (first-non-empty string from a set of keys, author login extraction, numeric field extraction) are likely to recur elsewhere as more GitHub API responses are processed.

Recommendation: Add a godoc comment on getFilteredItemStringField noting its generic nature for discoverability. If similar map-extraction patterns emerge in proxy or mcp packages, promote to internal/strutil/ or a new internal/maputil/ package.

Estimated effort: 30 min (comment only) to 2 hours (full extraction)


3. parseAndValidateIntEnv Partially Duplicates envutil.GetEnvInt

Files:

  • internal/config/config_env.go:15parseAndValidateIntEnv
  • internal/envutil/envutil.go:35GetEnvInt

The string-to-int conversion from environment variables is implemented in both functions. config_env.go already calls envutil.GetEnvString for the raw value, then re-implements strconv.Atoi with error handling that GetEnvInt also contains.

The difference is the return signature: config's version returns (int, bool, error) for explicit not-set detection and domain validation, while GetEnvInt returns a defaulted value.

Recommendation: Add GetEnvIntRaw(envKey string) (int, bool, error) to envutil that returns the triplet without defaulting, so parseAndValidateIntEnv can delegate the parse step:

// envutil/envutil.go
// GetEnvIntRaw reads envKey as an integer, returning (value, true, nil) when present and valid,
// (0, false, nil) when absent, or (0, false, err) when present but not a valid integer.
func GetEnvIntRaw(envKey string) (int, bool, error) { ... }

Estimated effort: 2–3 hours
Risk: Low — additive change, parseAndValidateIntEnv keeps its domain validation logic


Implementation Checklist

  • Extract WalkDockerEnvArgs helper in envutil and update logEnvPassthrough + ExpandEnvArgs to use it (Configure as a Go CLI tool #1 — highest impact)
  • Add discoverability comment to getFilteredItemStringField in server/difc_log.go (Lpcox/initial implementation #2 — quick win)
  • Add envutil.GetEnvIntRaw and update parseAndValidateIntEnv to delegate the parse step (Lpcox/initial implementation #3 — medium effort)
  • Update AGENTS.md if envutil public API changes are made

References: §26126105312

Generated by Semantic Function Refactoring · ● 1.1M ·

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions