Skip to content

[refactor] Semantic Function Clustering Analysis #5966

@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. The codebase is overall well-organized, with clear package boundaries and consistent naming conventions. The internal/logger package already uses a generic initLogger[T] helper in common.go to reduce repetition across logger types. However, three actionable refactoring opportunities were identified.


Analysis Metadata

  • Total Go files analyzed: 126 (excluding *_test.go)
  • Total top-level functions cataloged: 807
  • Function clusters identified: ~20 (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. Logger Convenience Functions: Naming Inconsistency Creates Ambiguity

Files: internal/logger/file_logger.go, internal/logger/markdown_logger.go, internal/logger/server_file_logger.go

The logger package exposes three families of level-convenience functions with inconsistent naming:

File logger Markdown logger Server file logger
LogInfo(category, ...) LogInfoToMarkdown(category, ...) LogInfoToServer(serverID, category, ...)
LogWarn(category, ...) LogWarnToMarkdown(category, ...) LogWarnToServer(serverID, category, ...)
LogError(category, ...) LogErrorToMarkdown(category, ...) LogErrorToServer(serverID, category, ...)
LogDebug(category, ...) LogDebugToMarkdown(category, ...) LogDebugToServer(serverID, category, ...)

The file logger functions (LogInfo, LogWarn, etc.) lack a destination suffix, while the other two families use ToMarkdown and ToServer. Callers cannot tell from the name alone which sink LogInfo targets — there's an implicit asymmetry.

Recommendation: Either rename the file-logger family to LogInfoToFile/LogWarnToFile/... (surfacing the sink explicitly), or document clearly in AGENTS.md/godoc that the un-suffixed forms always write to the file logger. If rename is chosen, a deprecation alias can ease the transition.

Estimated effort: 1–2 hours (rename + grep for all call sites)


2. parseAndValidateIntEnv Partially Duplicates envutil.GetEnvInt

Files: internal/config/config_env.go:15, internal/envutil/envutil.go:35

config_env.go contains a private parseAndValidateIntEnv helper that reads an env var as a string, converts it to int, and applies a domain-specific validation callback. envutil.GetEnvInt already performs string-to-int conversion with error handling.

// config/config_env.go
func parseAndValidateIntEnv(envKey string, validate func(int) *rules.ValidationError) (int, bool, error) {
    raw := envutil.GetEnvString(envKey, "")  // already uses envutil!
    value, err := strconv.Atoi(raw)         // duplicates envutil.GetEnvInt internals
    ...
    if validationErr := validate(value); validationErr != nil { ... }
}

// envutil/envutil.go
func GetEnvInt(envKey string, defaultValue int) int {
    if envValue := os.Getenv(envKey); envValue != "" {
        if value, err := strconv.Atoi(envValue); err == nil && value > 0 { ... }
    }
    return defaultValue
}

The key difference is that parseAndValidateIntEnv returns (int, bool, error) (present/valid/error triplet) while GetEnvInt returns a defaulted value. The config package's need for explicit not-set detection and domain validation is real, but the int-parsing logic itself is repeated.

Recommendation: Consider adding a GetEnvIntRaw(envKey string) (int, bool, error) variant to envutil that returns the triplet without defaulting, allowing parseAndValidateIntEnv to delegate the parse step. This consolidates int-from-env parsing in a single place.

Estimated effort: 2–3 hours


3. Outlier: Generic JSON Map Extraction Helpers in server/difc_log.go

File: internal/server/difc_log.go

This file contains DIFC-specific filtering log helpers alongside three generic map[string]interface{} extraction utilities:

// Generic enough to live 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 { ... }

While currently only used in DIFC log formatting, these map extraction patterns (getFilteredItemStringField in particular) are candidates to appear in other places that deal with untyped JSON responses. Having them buried in server/difc_log.go makes them invisible to other authors.

Recommendation: If similar extraction patterns emerge elsewhere, consider moving getFilteredItemStringField to internal/strutil/ or a new internal/maputil/ package. For now, add a comment noting the generic nature to ease future discoverability.

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


Implementation Checklist

  • Decide on logger naming strategy (add ToFile suffix or document LogInfo targets file)
  • Optionally add envutil.GetEnvIntRaw to consolidate int-env parsing
  • Add discoverability comment to getFilteredItemStringField in server/difc_log.go
  • Update AGENTS.md log-function table if naming changes are made

References: §26061146674

Generated by Semantic Function Refactoring · ● 1.4M ·

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