Skip to content

[refactor] Semantic function clustering analysis: split the compiler_types.go grab-bag; codebase otherwise well-factoredย #43058

Description

@github-actions

๐Ÿ”ง Semantic Function Clustering Analysis

Analysis of github/gh-aw โ€” non-test .go files under pkg/ (focus: pkg/workflow, pkg/cli, pkg/parser).

Executive summary

The codebase is already well-organized. Files largely follow the one-file-per-feature rule, helper functions are domain-scoped (engine_helpers.go, git_helpers.go, map_helpers.go, ...) rather than scattered, and there is a mature set of dedicated utility packages (stringutil, sliceutil, setutil, jsonutil, gitutil, envutil, errorutil, fileutil, typeutil, semverutil, repoutil, syncutil, timeutil). Most cross-file name collisions turned out to be intentional, documented delegation shims or _wasm.go build-tag variants โ€” not real duplication.

The one clearly actionable structural finding is the compiler_types.go grab-bag (65 KB, largest source file in the repo). Everything else is low priority.

Key metrics

Metric Value
Non-test .go files in pkg/ ~700 (995 incl. tests)
Files in pkg/workflow / pkg/cli / pkg/parser 425 / 337 / 43
Cross-file duplicate top-level func names (main pkgs) 5, all benign (shims / different signatures)
Genuine structural finding 1 (compiler_types.go)

Primary finding โ€” compiler_types.go is a grab-bag (Priority 1)

pkg/workflow/compiler_types.go (65 KB, 52 funcs, 12 types) mixes several unrelated concerns in one file:

  1. The functional-options pattern: CompilerOption + 7 With* constructors
  2. The Compiler struct, NewCompiler, and ~40 Set*/Get* accessor methods
  3. Multiple unrelated config type definitions: WorkflowData, SafeOutputsConfig, BaseSafeOutputConfig, SafeOutputMessagesConfig, MentionsConfig, SecretMaskingConfig, SkipIfMatchConfig, SkipIfNoMatchConfig, SkipIfCheckFailingConfig

Outlier detail โ€” safe-output config types are in the wrong file. A dedicated safe_outputs_config.go (42 KB) exists but defines only a single type (SafeOutputStepConfig), while the core safe-output types (SafeOutputsConfig, BaseSafeOutputConfig, SafeOutputMessagesConfig, MentionsConfig, SecretMaskingConfig) live in compiler_types.go. These belong next to their behavior in safe_outputs_config.go.

WorkflowData is referenced by ~178 non-test files in pkg/workflow โ€” it is a central domain type and arguably deserves its own workflow_data.go.

Suggested split (mechanical, low risk)
  • compiler_options.go โ† CompilerOption, the 7 With* functions, NewCompiler
  • compiler_accessors.go โ† the ~40 Compiler Set*/Get* methods
  • safe_outputs_config.go (existing) โ† move SafeOutputsConfig, BaseSafeOutputConfig, SafeOutputMessagesConfig, MentionsConfig, SecretMaskingConfig
  • workflow_data.go โ† WorkflowData (+ its PinContext method) and the SkipIf*Config structs
  • compiler_types.go (remaining) โ† just the Compiler struct + FileCreationTracker interface

All moves are within-package, so no import churn. Estimated effort: 1โ€“2 hours; benefit: clearer navigation, smaller diff surface on the hottest file.

Secondary findings (Priority 3 โ€” likely intentional, verify only)

Cross-package functional duplicates that differ by type

These share a name but have different signatures and live in different packages โ€” they operate on different representations of the same idea, so consolidation would require a shared abstraction and may not be worth it:

  • hasCopilotRequestsWritePermission โ€” pkg/cli/workflow_secrets.go (takes frontmatter map[string]any) vs pkg/workflow/permissions_operations.go (takes *WorkflowData)
  • resolveImportInputPath โ€” pkg/parser/import_field_extractor.go ((string, bool)) vs pkg/workflow/expression_extraction.go ((any, bool))

If the two permission checks are meant to enforce the same rule, consider a single helper in pkg/workflow that both call once frontmatter is parsed into WorkflowData.

Confirmed NON-issues (documented for transparency)

These name collisions were investigated and are correct as-is:

  • GetVersion โ€” pkg/cli/version.go is a documented thin delegate to pkg/workflow/version.go (single source of truth).
  • compileSchema โ€” both pkg/workflow/schema_utils.go and pkg/parser/schema_compiler.go are documented shims delegating to parser.CompileSchema.
  • ExecGH, RunGH, RunGHContext, RunGHCombined, ForceGHHostEnv, SetDefaultGHHost, getDefaultGHHost, findGitRoot, RunGitCombined, ResolveIncludePath, etc. โ€” these are _wasm.go build-tag variant pairs, not duplication.

Next actions

  • Split compiler_types.go as outlined above (Priority 1, mechanical)
  • Move safe-output config types into safe_outputs_config.go
  • (Optional) Decide whether the two hasCopilotRequestsWritePermission checks should share one implementation

Analysis metadata

  • Method: naming-pattern clustering + duplicate-name detection across non-test files, with _wasm.go build variants and delegation shims filtered out; Serena Go LSP available for follow-up symbol inspection.
  • Scope: pkg/workflow, pkg/cli, pkg/parser (primary); util packages surveyed.
  • Analysis date: 2026-07-03.
  • Existing [refactor] issues closed first: none were open.

Generated by ๐Ÿ”ง Semantic Function Refactoring ยท 163.9 AIC ยท โŒ– 14.9 AIC ยท โŠž 9.2K ยท โ—ท

  • expires on Jul 4, 2026, 4:11 PM UTC-08:00

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions