[refactor] Semantic Function Clustering Analysis: Refactoring Opportunities

[github-actions]

Executive Summary

Analysis of 146 non-test Go files across 20 packages (~940 functions). The codebase is well-organized with clean single-responsibility boundaries and good use of generics. Three concrete refactoring opportunities were found.

---

Analysis Metadata

• Total Go files analyzed: 146 (non-test, internal/)
• Total functions cataloged: ~940 across 20 packages
• Outlier functions: 0 (no functions misplaced in wrong files)
• Detection method: Static function inventory + naming pattern analysis + call-site tracing

---

Finding 1 — Logger Close*Logger() Boilerplate [Medium Impact]

Six package-level functions in internal/logger/ each contain a single line and are only referenced in registry.go. They are never called directly from production code anywhere else.

Function	File	Body
CloseGlobalLogger()	file_logger.go	return closeGlobalLogger(&globalLoggerMu, &globalFileLogger)
CloseJSONLLogger()	jsonl_logger.go	return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger)
CloseMarkdownLogger()	markdown_logger.go	return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger)
CloseServerFileLogger()	server_file_logger.go	return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger)
CloseToolsLogger()	tools_logger.go	return closeGlobalLogger(&globalToolsMu, &globalToolsLogger)
CloseObservedURLDomainsLogger()	observed_url_domains_logger.go	return closeGlobalLogger(&globalObservedURLDomainsMu, &globalObservedURLDomainsLogger)

Recommendation: Inline as anonymous closures directly in registry.go's globalLoggerClosers slice and remove the 6 named functions. Centralizes close logic in one place, removes ~30 lines of boilerplate.

// registry.go — consolidated
var globalLoggerClosers = []loggerCloseEntry{
    {name: "file logger",     close: func() error { return closeGlobalLogger(&globalLoggerMu, &globalFileLogger) }},
    {name: "JSONL logger",    close: func() error { return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger) }},
    {name: "markdown logger", close: func() error { return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger) }},
    // ... etc.
}

Effort: ~30 min | Risk: Low — update tests using individual Close*Logger to call CloseAllLoggers() instead.

---

Finding 2 — syncutil.GetOrCreate Naming Collision [Medium Impact]

The syncutil package exposes two GetOrCreate operations with the same name but fundamentally different interfaces:

Package-level function (syncutil/cache.go) — low-level, requires external mutex + map:

func GetOrCreate[K comparable, V any](mu *sync.RWMutex, cache map[K]V, key K, create func() (V, error)) (V, error)

TTLCache method (syncutil/ttl_cache.go) — self-contained, TTL-aware, create cannot error:

func (c *TTLCache[K, V]) GetOrCreate(key K, create func() V) V

Key differences: caller-managed mutex vs self-contained; create can error vs cannot; no TTL vs TTL eviction. Callers currently use both (e.g., session.go uses package function; routed.go uses TTLCache method), making it easy to choose the wrong abstraction.

Recommendation: Rename the package-level function to signal its low-level nature:

func MapGetOrCreate[K comparable, V any](mu *sync.RWMutex, cache map[K]V, key K, create func() (V, error)) (V, error)

Update 4 call sites: server/session.go, launcher/launcher.go, difc/agent.go, logger/server_file_logger.go.

Effort: ~1 hour | Risk: Low — mechanical rename.

---

Finding 3 — Private Log-Dispatch Triplet [Low Impact]

Three private functions in internal/logger/ follow the same structural pattern, each dispatching to a different global logger via withGlobalLogger:

• logWithLevel(level, category, format, args) → FileLogger (file_logger.go)
• logWithMarkdown(level, category, format, args) → MarkdownLogger (markdown_logger.go)
• logWithLevelAndServer(serverID, level, category, format, args) → ServerFileLogger (server_file_logger.go)

The existing global_state.go comment (lines 294–330) documents this pattern intentionally, and newLevelLoggerFuncs/newServerLevelLoggerFuncs generics already partially abstract it. This is lower priority.

Recommendation: Consider eliminating the intermediate dispatch layer by calling withGlobalLogger inline at the exported LogInfo/LogWarn level, or add a cross-reference comment pointing to the pattern documentation.

Effort: 1–2 hours | Risk: Medium.

---

What Was Found to Be Well-Organized

• config/: Clean decomposition across 16 files; already uses initLogger[T] + loggerFactory[T] generics for init.
• server/: Each file has a focused concern. applyIfConfigured/applyAuthIfConfigured/applyHMACIfConfigured abstraction is clean.
• difc/: Clean separation of evaluator, labels, capabilities, agent registry, path labels.
• proxy/: GraphQL rewrite, route matching, response transform, and TLS appropriately separated.
• strutil/: Utility functions centralized with no scattering detected.
• tracing/: genai_attrs.go as dedicated constants file is idiomatic. Config resolution, fanout, span helpers cleanly separated.

Implementation Checklist

• Finding 1: Inline closers in registry.go; delete 6 Close*Logger wrapper functions; update tests
• Finding 2: Rename syncutil.GetOrCreate → MapGetOrCreate; update 4 call sites + tests
• Finding 3 (optional): Eliminate or document private dispatch functions

Generated by Semantic Function Refactoring · 113.2 AIC · ⊞ 9.3K · ◷