Skip to content

[refactor] Semantic function clustering: scattered compilation stats, mixed concerns in compile_post_processing.go, and wrapper chain in co [Content truncated due to length] #27811

@github-actions

Description

@github-actions

Semantic analysis of 720 non-test Go files across pkg/ identified three concrete refactoring opportunities in pkg/cli/. The pkg/workflow/ package (350 files) uses Go generics effectively and is well-organized; the patterns below are specific to the compile_*.go cluster.


Overview

  • Total Go files analyzed: 720 (non-test, in pkg/)
  • Packages: workflow (350), cli (259), parser (40), console (25), agentdrain (10), utility packages (36)
  • Issues found: 3 (all in pkg/cli/compile_*.go)
  • No issues found in: pkg/workflow/ (update entity helpers, string utils, config helpers, and validation files are well-organized), pkg/parser/, utility packages

Issue 1: Compilation Stats Logic Scattered Across 4 Files

Severity: Medium

The CompilationStats type and its related functions are spread across four separate files with no clear ownership:

File Content
compile_config.go:46 type CompilationStats struct { ... } — type definition
compile_file_operations.go:275 trackWorkflowFailure(stats *CompilationStats, ...) — mutation
compile_file_operations.go:288 printCompilationSummary(stats *CompilationStats) — display
compile_stats.go:95 displayStatsTable(statsList []*WorkflowStats) — display
compile_post_processing.go:95 collectWorkflowStatisticsWrapper(...) — collection

compile_stats.go already exists as the natural home for this logic, but contains only WorkflowStats-related functions (collectWorkflowStats, displayStatsTable), not CompilationStats functions. The two stats types and their functions live in different files.

Recommendation: Consolidate all CompilationStats operations (trackWorkflowFailure, printCompilationSummary, collectWorkflowStatisticsWrapper) into compile_stats.go alongside the type definition (or co-locate type + operations).


Issue 2: Mixed Concerns in compile_post_processing.go

Severity: Medium

The file's header comment states its purpose is "post-processing operations (generating Dependabot manifests and maintenance workflows)", but it also contains three functions that fall outside this scope:

compile_post_processing.go:114  func updateGitAttributes(...)   // git infrastructure
compile_post_processing.go:147  func saveActionCache(...)        // cache management
compile_post_processing.go:171  func getAbsoluteWorkflowDir(...) // pure path utility

The first two perform distinct operations (.gitattributes generation, action cache persistence) that have no conceptual relation to "post-processing of compiled manifests". The third is a generic utility.

Recommendation:

  • Move updateGitAttributes and saveActionCache to compile_pipeline.go or a new compile_infrastructure.go (they're infrastructure concerns called from the pipeline)
  • Move getAbsoluteWorkflowDir to compile_file_operations.go or inline at the single call site

Issue 3: Unnecessary Wrapper Chain in compile_batch_operations.go

Severity: Low

Each public batch tool entry point creates a 3-level call chain:

RunActionlintOnFiles()       // public, only adds empty-slice guard
  → runBatchActionlint()     // private, delegates to generic runner
    → runBatchLockFileTool() // generic, calls actual runner function
      → runActionlintOnFiles() // actual implementation

The same pattern repeats for RunZizmorOnFiles, RunPoutineOnDirectory, and RunRunnerGuardOnDirectory. The intermediate private wrappers (runBatchActionlint, runBatchZizmor, runBatchPoutine, runBatchRunnerGuard) each do nothing except pass their arguments to runBatchLockFileTool.

Current pattern (lines 42–137)
// Level 1: public entry with empty-slice guard
func RunActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error {
    if len(lockFiles) == 0 { return nil }
    return runBatchActionlint(lockFiles, verbose, strict)
}

// Level 2: private wrapper — does nothing but forward
func runBatchActionlint(lockFiles []string, verbose bool, strict bool) error {
    return runBatchLockFileTool("actionlint", lockFiles, verbose, strict, runActionlintOnFiles)
}

// Level 3: generic runner
func runBatchLockFileTool(toolName string, lockFiles []string, verbose bool, strict bool, runner func(...) error) error { ... }

Recommendation: Collapse levels 1 and 2. The public Run* function can call runBatchLockFileTool directly with the tool-specific runner, eliminating 4 single-line pass-through functions:

func RunActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error {
    return runBatchLockFileTool("actionlint", lockFiles, verbose, strict, runActionlintOnFiles)
}

What Was Checked But Found Clean

  • pkg/workflow/update_entity_helpers.go: Excellent use of Go generics (parseUpdateEntityConfigTyped[T any]) — no changes needed
  • pkg/workflow/strings.go vs pkg/stringutil/: Clear intentional separation documented inline — no duplication
  • pkg/workflow/strings.go:ValidateHeredocContent/ValidateHeredocDelimiter: These are tightly coupled to GenerateHeredocDelimiterFromSeed in the same file; colocation is justified
  • codemod_*.go files: Well-organized factory pattern, consistent structure
  • pkg/stringutil, pkg/fileutil, pkg/sliceutil, pkg/timeutil, pkg/typeutil: All focused, no cross-package duplication

Analysis Metadata

  • Run: §24776123192
  • Files analyzed: 720 non-test .go files in pkg/
  • Method: Serena LSP semantic analysis + naming pattern analysis
  • Date: 2026-04-22

Generated by Semantic Function Refactoring · ● 651.5K ·

  • expires on Apr 24, 2026, 11:43 AM UTC

Metadata

Metadata

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