-
Notifications
You must be signed in to change notification settings - Fork 340
[refactor] Semantic Function Clustering Analysis: Refactoring Opportunities #25279
Description
Automated semantic analysis of all non-test Go source files in pkg/ (665 files across 20 packages). The analysis used Serena LSP tooling and pattern search to identify outlier functions and near-duplicate implementations.
Executive Summary
- Total Go files analyzed: 665 (across 20 packages)
- Primary packages:
pkg/workflow(322 files),pkg/cli(234 files),pkg/parser(40 files) - Issues found: 3 actionable refactoring opportunities + 2 architectural observations
- Overall assessment: Code is well-organized with strong domain separation. Most "duplicates" are intentional aliases documented inline. Three genuine improvements were found.
Issue 1: DependencyGraph.findGitRoot() Duplicates gitutil.FindGitRoot()
Priority: High — clear outlier function
File: pkg/cli/dependency_graph.go:262–282
DependencyGraph.findGitRoot() implements its own directory-walking loop to locate .git:
func (g *DependencyGraph) findGitRoot() string {
dir := g.workflowsDir
for {
gitDir := filepath.Join(dir, ".git")
if _, err := os.Stat(gitDir); err == nil {
return dir
}
parent := filepath.Dir(dir)
if parent == dir { break }
dir = parent
}
return g.workflowsDir // Fallback
}The codebase already has a dedicated utility for this:
pkg/gitutil/gitutil.go—FindGitRoot()usinggit rev-parse --show-toplevelpkg/workflow/git_helpers.go:54—findGitRoot()that correctly delegates togitutil.FindGitRoot()pkg/cli/git.go:25—findGitRootForPath(path)usinggit -C dir rev-parse --show-toplevel
The DependencyGraph implementation uses a different algorithm (filesystem walk vs. git command) and has a silent fallback to g.workflowsDir, which could mask errors and return incorrect results in git worktrees or non-standard repository layouts.
Recommendation: Replace the manual implementation with a call to gitutil.FindGitRoot() (or findGitRootForPath), retaining the g.workflowsDir fallback if needed but making it explicit.
Estimated effort: 15–30 minutes
Risk: Low (pure refactor, behavior preserved)
Issue 2: map_helpers.go Package-Alias Wrappers Are Migration Debt
Priority: Medium — documented technical debt
File: pkg/workflow/map_helpers.go:58–116
Five functions in this file are explicit one-liner aliases to pkg/typeutil/convert.go:
Alias in map_helpers.go |
Delegates to typeutil |
|---|---|
parseIntValue(value any) |
typeutil.ParseIntValue |
safeUint64ToInt(u uint64) |
typeutil.SafeUint64ToInt |
safeUintToInt(u uint) |
typeutil.SafeUintToInt |
ConvertToInt(val any) |
typeutil.ConvertToInt |
ConvertToFloat(val any) |
typeutil.ConvertToFloat |
The file already documents this: "For new code outside this package, prefer using typeutil.ConvertToInt directly." However, callers inside the workflow package still use the aliases:
claude_logs.go—ConvertToInt,ConvertToFloat(multiple calls)mcp_scripts_parser.go—safeUint64ToInt(2 calls)frontmatter_extraction_metadata.go—safeUintToInt,safeUint64ToInt(4 calls)publish_artifacts.go—parseIntValue(4 calls)safe_outputs_config.go—parseIntValue(1 call)publish_assets.go—parseIntValue(1 call)
Recommendation: Migrate internal callers to use typeutil.* directly and remove the alias wrappers. This eliminates indirection and makes the dependency explicit.
Estimated effort: 1–2 hours (mechanical find-and-replace)
Risk: Low (type signatures are identical)
Issue 3: Near-Duplicate Bool Extractors Across Package Boundary
Priority: Medium — functional duplication
Two functions perform identical operations (extract a bool from map[string]any by key, return false as default):
pkg/parser/frontmatter_hash.go:23:
func parseBoolFromFrontmatter(m map[string]any, key string) bool {
if m == nil { return false }
if v, ok := m[key]; ok {
b, _ := v.(bool)
return b
}
return false
}pkg/workflow/config_helpers.go:184:
func ParseBoolFromConfig(m map[string]any, key string, log *logger.Logger) bool {
if value, exists := m[key]; exists {
if boolValue, ok := value.(bool); ok {
return boolValue
}
}
return false
}The logic is identical. The workflow version adds optional logging. The parser version handles nil map.
Recommendation: Add a ParseBool(m map[string]any, key string) bool function to pkg/typeutil/convert.go (which already owns ConvertToInt, ConvertToFloat, ParseIntValue). Both callers can then delegate to it.
Estimated effort: 30–60 minutes
Risk: Low
Observation A: mergeUnique in runtime_definitions.go
File: pkg/workflow/runtime_definitions.go:221
mergeUnique deduplicates-while-merging two string slices. pkg/sliceutil has Deduplicate[T] but not a MergeUnique variant. This is not a bug — the semantics differ — but it could be an opportunity to add sliceutil.MergeUnique if the pattern recurs elsewhere.
Affected: 1 call site; not worth immediate action.
Observation B: Sanitization Split Between workflow/strings.go and stringutil/
General-purpose sanitization functions live in pkg/stringutil/sanitize.go (SanitizeErrorMessage, SanitizeParameterName, SanitizePythonVariableName, SanitizeToolID, SanitizeForFilename).
Workflow-specific sanitization lives in pkg/workflow/strings.go (SanitizeName with options, SanitizeWorkflowName, SanitizeIdentifier, SanitizeWorkflowIDForCacheKey).
This split is intentional and appropriate — the workflow-specific functions have domain-specific defaults and use a SanitizeOptions struct not needed in the general package. No action required.
Implementation Checklist
- Issue 1: Replace
DependencyGraph.findGitRoot()(dependency_graph.go:262) withgitutil.FindGitRoot() - Issue 2: Migrate
workflowpackage callers off map_helpers aliases → usetypeutil.*directly; remove the 5 wrapper functions - Issue 3: Add
ParseBooltopkg/typeutil/convert.go; updateparseBoolFromFrontmatterandParseBoolFromConfigto delegate
Analysis Metadata
- Total Go files analyzed: 665
- Packages analyzed: 20 (
pkg/workflow,pkg/cli,pkg/parser,pkg/stringutil,pkg/typeutil,pkg/sliceutil,pkg/gitutil,pkg/console,pkg/constants,pkg/agentdrain, and 10 single-file utility packages) - Detection method: Serena LSP semantic analysis + pattern search + function signature comparison
- Workflow run: §24133171752
- Analysis date: 2026-04-08
Generated by Semantic Function Refactoring · ● 773K · ◷
- expires on Apr 10, 2026, 11:43 AM UTC