Skip to content

[refactor] Semantic Function Clustering Analysis: Code Organization and Duplication Opportunities #23675

Description

@github-actions

Overview

Semantic analysis of 621 non-test Go source files across pkg/cli (218 files), pkg/workflow (311 files), pkg/parser (40 files), and utility packages. Three concrete refactoring opportunities were identified with high confidence, plus additional medium-priority items.

Overall the codebase is well-organized — validation files, helper files, and safe_outputs sub-modules are clearly separated by concern. This report focuses on areas with actionable improvement potential.


Critical Findings

1. Duplicate semanticVersion Struct in pkg/cli/semver.go

pkg/cli/semver.go defines a local semanticVersion struct (lines 13–19) that is structurally identical to semverutil.SemanticVersion in pkg/semverutil/semverutil.go:

// pkg/cli/semver.go (local duplicate)
type semanticVersion struct {
    major int
    minor int
    patch int
    pre   string
    raw   string
}
// pkg/semverutil/semverutil.go (canonical)
type SemanticVersion struct {
    Major int
    Minor int
    Patch int
    Pre   string
    Raw   string
}

The parseVersion() function in pkg/cli/semver.go already delegates to semverutil.ParseVersion() but then manually re-maps all fields into the local struct instead of returning *semverutil.SemanticVersion directly. The cli package already imports semverutil (see import at line 7), so the dependency exists.

Recommendation: Remove type semanticVersion struct from pkg/cli/semver.go and use *semverutil.SemanticVersion directly. Update isPreciseVersion() and isNewer() as extension methods or package-level functions in semverutil.

Estimated effort: ~1 hour
Impact: Removes a struct definition duplication, aligns canonical type usage


2. findGitRoot() Thin Wrappers — Redundant Indirection

gitutil.FindGitRoot() is already defined in pkg/gitutil/gitutil.go:70. Two packages create thin local wrappers:

File Function Signature difference
pkg/gitutil/gitutil.go FindGitRoot() (string, error) — canonical
pkg/cli/git.go:25 findGitRoot() (string, error) — direct passthrough, zero logic
pkg/workflow/git_helpers.go:54 findGitRoot() string — absorbs error, logs on failure

pkg/cli/git.go:findGitRoot() is a one-liner passthrough (return gitutil.FindGitRoot()) that adds no value. Call sites in pkg/cli should call gitutil.FindGitRoot() directly.

pkg/workflow/git_helpers.go:findGitRoot() has slight divergence (returns empty string on error instead of propagating it), which may be intentional — this case should be evaluated separately before any change.

Recommendation: Remove findGitRoot() from pkg/cli/git.go and update call sites to use gitutil.FindGitRoot() directly. Evaluate whether pkg/workflow/git_helpers.go:findGitRoot() can also be eliminated or if the error-swallowing behavior is required.

Estimated effort: ~30 minutes
Impact: Removes unnecessary indirection; reduces number of functions to maintain


3. Config Parsing Utilities — Partial Consolidation Opportunity

pkg/workflow/config_helpers.go defines a set of generic config-parsing utilities (ParseStringArrayFromConfig, ParseBoolFromConfig, parseConfigScaffold[T], etc.) intended to be reused across workflow step implementations. However, similar parsing patterns appear in several entity-specific helper files:

  • pkg/workflow/update_entity_helpers.go — parseUpdateEntityConfig, parseUpdateEntityBase, parseUpdateEntityBoolField, parseUpdateEntityStringBoolField, parseUpdateEntityConfigWithFields, parseUpdateEntityConfigTyped
  • pkg/workflow/update_issue_helpers.go — parseUpdateIssuesConfig
  • pkg/workflow/update_discussion_helpers.go — parseUpdateDiscussionsConfig
  • pkg/workflow/update_pull_request_helpers.go — parseUpdatePullRequestsConfig
  • pkg/workflow/close_entity_helpers.go — parseCloseEntityConfig, parseCloseIssuesConfig, parseClosePullRequestsConfig, parseCloseDiscussionsConfig

The entity-specific parsers (parseUpdateIssuesConfig, etc.) delegate to the generic parseUpdateEntityConfig, so the top-level pattern is sound. The question is whether the generic field parsers in update_entity_helpers.go (parseUpdateEntityBoolField, etc.) duplicate logic already in config_helpers.go (ParseBoolFromConfig).

Recommendation: Audit whether parseUpdateEntityBoolField and parseUpdateEntityStringBoolField can be replaced with ParseBoolFromConfig from config_helpers.go. If they share the same logic, remove the duplicates and unify.

Estimated effort: ~2 hours
Impact: Reduces number of parallel field-parsing implementations; makes the "canonical" path clearer for future implementors


Medium-Priority Findings

4. Validation File WASM Variants

Three validation domains have _wasm.go companion files:

  • npm_validation.go / npm_validation_wasm.go
  • pip_validation.go / pip_validation_wasm.go
  • docker_validation.go / docker_validation_wasm.go

If the WASM variants are stub/no-op implementations of the same interface, they may be consolidatable using build tags or a shared interface with platform-specific implementations. If the differences are substantive, no action is needed.

Recommendation: Review if WASM validation files implement the same function signatures with platform-appropriate behavior. If so, consider if a feature detection or interface pattern would be cleaner than file duplication.

5. Error Formatting — Multiple Independent Implementations

Error formatting is implemented independently in several locations:

  • pkg/workflow/compiler_error_formatter.go — compiler-specific (formatCompilerError, formatCompilerErrorWithPosition)
  • pkg/workflow/workflow_errors.go — error type definitions and NewErrorCollector()
  • pkg/console/console.go — display-level (FormatError, FormatErrorMessage, FormatErrorChain)
  • pkg/parser/yaml_error.go / import_error.go — domain-specific formatters
  • pkg/cli/compile_output_formatter.go — JSON output formatting

The separation between error types, error formatting for display, and domain-specific error enrichment is clear in principle, but the formatting functions are not coordinated. This is unlikely to cause bugs but can lead to inconsistent user-facing error messages.

Recommendation: Establish a documented convention for the error formatting layers (type → enrich → display). No immediate code changes required unless inconsistent error output is observed.


What Was Analyzed Well (No Action Needed)

  • Sanitize functions: pkg/stringutil/sanitize.go handles character validity; pkg/workflow/strings.go handles format standardization. These serve distinct purposes and are correctly separated.
  • Semver utilities: pkg/semverutil/semverutil.go is the canonical implementation; pkg/workflow/semver.go correctly delegates to it. Only pkg/cli/semver.go has the struct duplication noted above.
  • Validation organization: 56 validation files in pkg/workflow are well-segregated by domain. The validation.go + validation_helpers.go + domain-specific *_validation.go pattern is clean.
  • Entity helpers registry pattern: close_entity_helpers.go and update_entity_helpers.go correctly implement a generic-over-specific delegation pattern. This is intentional, not duplication.
  • Helper file naming: 15+ *_helpers.go files are clearly named and domain-scoped. No scattered helpers were found.

Analysis Metadata

Metric Value
Total non-test Go files analyzed 621
Primary packages pkg/cli, pkg/workflow, pkg/parser, pkg/stringutil, pkg/gitutil, pkg/semverutil
Confirmed duplicate structs 1 (semanticVersion)
Thin wrappers to evaluate 1 (cli.findGitRoot)
Parse pattern consolidation candidates ~6 functions
Detection method Serena LSP semantic analysis + naming pattern analysis
Analysis date 2026-03-31

References: §23795060950

Generated by Semantic Function Refactoring · ◷

  • expires on Apr 2, 2026, 11:37 AM UTC

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions