Skip to content

[refactor] Semantic Function Clustering Analysis: Code Organization Opportunities #24072

Description

@github-actions

Automated semantic analysis of the Go codebase identified refactoring opportunities across 635 non-test source files (313 in pkg/workflow/, 224 in pkg/cli/) cataloging 2,570 functions.

Executive Summary

  • Total Go files analyzed: 635 (non-test)
  • Total functions cataloged: 2,570 (1,581 in workflow, 989 in cli)
  • Function clusters identified: 8 semantic groups
  • Outliers / mixed-concern files: 3 confirmed
  • Duplicate/near-duplicate functions: 3 instances
  • Detection method: Serena LSP + naming pattern analysis (§23898267556)

Critical Issues

Issue 1 — mcp_server_helpers.go Mixes 4 Distinct Concerns

File: pkg/cli/mcp_server_helpers.go

The file conflates four unrelated domains:

Function Domain
newMCPError(), mcpErrorData() Error handling / serialization
boolPtr() Generic Go pointer utility
getRepository(), queryActorRole() GitHub repository / GraphQL API
hasWriteAccess(), checkActorPermission(), validateMCPWorkflowName() Authorization / permissions

Recommendation: Split into purpose-named files:

  • mcp_error.go — error helpers
  • mcp_permissions.gohasWriteAccess, checkActorPermission, queryActorRole
  • mcp_repository.gogetRepository
  • Move boolPtr() to pkg/cli/util.go or use lo.ToPtr / inline it (it has only one call site)

Estimated effort: 1–2 hours
Impact: Clearer file navigation; easier to find permission-related code during security reviews


Issue 2 — compiler_yaml_helpers.go Mixes Step Conversion, Lookups, and Generation

File: pkg/workflow/compiler_yaml_helpers.go (439 lines, 12 functions)

Function Concern
ContainsCheckout(), GetWorkflowIDFromPath() Query / utility checks
ConvertStepToYAML(), unquoteUsesWithComments() YAML ↔ map conversion
getInstallationVersion(), getDefaultAgentModel(), versionToGitRef() Version / model resolution lookups
generateCheckoutActionsFolder(), generateSetupStep(), generateSetRuntimePathsStep(), generateRestoreActionsSetupStep() Step generation (builder pattern)
renderStepFromMap() YAML rendering

Recommendation: Split into:

  • compiler_yaml_step_conversion.goConvertStepToYAML, unquoteUsesWithComments, renderStepFromMap
  • compiler_yaml_lookups.gogetInstallationVersion, getDefaultAgentModel, versionToGitRef
  • compiler_yaml_step_generation.go — the 4 generate* methods

Estimated effort: 2–3 hours
Impact: Easier to locate specific logic; step generation (which grows most) separated from stable lookup code


Issue 3 — Three Overlapping Duration Formatters

Three duration formatting functions implement similar logic in different units/files, without cross-referencing pkg/timeutil:

Function File Input Behavior
timeutil.FormatDuration(d time.Duration) pkg/timeutil/format.go time.Duration ns → µs → ms → s → min
FormatDurationMs(ms int) pkg/cli/token_usage.go milliseconds int ms → s → min → h
formatDurationNs(ns int64) pkg/cli/audit_cross_run_render.go nanoseconds int64 delegates to d.Round(time.Second).String()

Recommendation:

  • Extend pkg/timeutil/format.go with FormatDurationMs(ms int) string and FormatDurationNs(ns int64) string
  • Update pkg/cli/token_usage.go and pkg/cli/audit_cross_run_render.go to use the shared package

Estimated effort: 1 hour
Impact: Single place to update time display logic; consistent formatting across audit/token/health outputs


Detailed Function Clusters

Cluster: Validation files and wasm stubs (intentional, no action needed)

pkg/workflow/ contains 7 *_wasm.go files providing WebAssembly build-tag stubs:

  • npm_validation_wasm.go — stub for validateNpxPackages()
  • pip_validation_wasm.go — stub for pip validation
  • docker_validation_wasm.go — stub for docker validation
  • dependabot_wasm.go — stub for GenerateDependabotManifests()
  • git_helpers_wasm.go, github_cli_wasm.go, repository_features_validation_wasm.go

Each file uses //go:build js || wasm tags correctly. The non-wasm counterpart uses //go:build !js && !wasm. This is the correct Go pattern — no change needed.

Cluster: update_*_helpers.go files (intentional, no action needed)

update_issue_helpers.go, update_pull_request_helpers.go, update_discussion_helpers.go each contain one config type and one parse function. They delegate to the shared generic parseUpdateEntityConfigTyped() in update_entity_helpers.go. This pattern is explicitly documented in update_entity_helpers.go and follows the developer conventions. No change needed.

Cluster: build*Summary functions in logs_report.go

Nine summary builder functions in pkg/cli/logs_report.go all follow the signature buildXxxSummary(processedRuns []ProcessedRun) → *SummaryType:

  • buildToolUsageSummary() (line 377)
  • buildMissingToolsSummary() (line 487)
  • buildMissingDataSummary() (line 531)
  • buildMCPFailuresSummary() (line 575)
  • buildAccessLogSummary() (line 670)
  • buildFirewallLogSummary() (line 704)
  • buildRedactedDomainsSummary() (line 749)
  • buildMCPToolUsageSummary() (line 786)
  • buildCombinedErrorsSummary() (line 927)

These are all in one file, so discoverability is fine. However, logs_report.go is very large and mixes report building (aggregation) with report rendering. Low priority.

Cluster: audit render functions (well-organized, no action needed)

Render functions are correctly grouped by feature file:

  • audit_report_render.go — 30+ functions for the main audit report
  • audit_diff_render.go — 12 functions for diff output
  • audit_cross_run_render.go — 3 functions for cross-run output

The render* naming convention is consistent. No change needed.

Cluster: compile_validation.go mixed concerns (minor)

pkg/cli/compile_validation.go exports three public linting runner wrappers alongside validation-specific helpers:

func RunActionlintOnFiles(...)  // delegates to actionlint.go private function
func RunZizmorOnFiles(...)      // delegates to zizmor
func RunPoutineOnDirectory(...) // delegates to poutine

These public entry points are thin wrappers and could logically live in actionlint.go / compile_batch_operations.go. Minor concern — not blocking.


Refactoring Recommendations

Priority 1 — High Impact (clear organizational benefit)

  1. Split mcp_server_helpers.go into domain-named files
    Effort: 1–2 hours | Benefit: Security-sensitive permission code easier to audit

  2. Split compiler_yaml_helpers.go into step-conversion / lookups / generation
    Effort: 2–3 hours | Benefit: Generation logic (most volatile) separated from stable helpers

  3. Consolidate duration formatters into pkg/timeutil
    Effort: 1 hour | Benefit: Single source of truth for time display formatting

Priority 2 — Medium Impact (structural hygiene)

  1. Move RunActionlintOnFiles / RunZizmorOnFiles / RunPoutineOnDirectory from compile_validation.go to compile_batch_operations.go
    Effort: <1 hour | Benefit: compile_validation.go name matches its content more accurately

Implementation Checklist

  • Review Priority 1 findings and confirm scope
  • Split mcp_server_helpers.go into domain-named files
  • Split compiler_yaml_helpers.go by concern
  • Extend pkg/timeutil with FormatDurationMs / FormatDurationNs and update callers
  • Consider moving linting runners out of compile_validation.go
  • Verify no compilation errors after moves

References:

Generated by Semantic Function Refactoring ·

  • expires on Apr 4, 2026, 11:39 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