-
Notifications
You must be signed in to change notification settings - Fork 312
Description
Overview
Semantic function clustering analysis was performed on all non-test Go files in pkg/ (597 files, 573 with functions, ~2,761 total function definitions). The analysis identified several concrete refactoring opportunities ranging from duplicate functions to oversized helper files.
Packages analyzed: pkg/cli/ (198 files), pkg/workflow/ (307 files), pkg/parser/ (42 files), pkg/console/ (24 files), and 9 utility packages.
Critical Findings
1 exact/near-duplicate, 1 pair of near-duplicate functions, 1 large file exceeding single-responsibility principles, and several patterns of functional duplication in config generation.
Issue 1: Duplicate Integer Conversion — safeUintToInt Wrapper
Severity: Medium
Files:
pkg/workflow/map_helpers.go:73— definessafeUint64ToInt(u uint64) intpkg/workflow/frontmatter_extraction_metadata.go:146— defines a thin wrappersafeUintToInt(u uint) int
The wrapper in frontmatter_extraction_metadata.go exists solely to adapt uint → uint64:
// frontmatter_extraction_metadata.go:145-146
func safeUintToInt(u uint) int { return safeUint64ToInt(uint64(u)) }This wrapper is used only within that file (lines 167, 209). The safeUint64ToInt function itself is also called directly in the same file (lines 169, 211), resulting in two nearly-identical conversion paths in one file.
Recommendation: Add safeUintToInt(u uint) int directly to pkg/workflow/map_helpers.go alongside safeUint64ToInt. Remove it from frontmatter_extraction_metadata.go. This consolidates all safe integer conversion utilities in one place.
Issue 2: Near-Duplicate Integer Parsing — parseIntValue vs ConvertToInt
Severity: Medium
File: pkg/workflow/map_helpers.go
Two functions in the same file perform overlapping work:
// parseIntValue: handles int, int64, float64 — returns (value, ok)
func parseIntValue(value any) (int, bool)
// ConvertToInt: handles int, int64, float64, AND string — returns int (0 on failure)
func ConvertToInt(val any) intConvertToInt is a superset of parseIntValue (adds string parsing) but uses different error handling (returns 0 vs returns false). The documentation in map_helpers.go lists both as siblings without explaining when to use each.
Recommendation: Refactor so ConvertToInt delegates to parseIntValue for numeric types, only adding string-parse logic on top. Or unify into a single function with an options variant (e.g., ConvertToIntFromString bool). At minimum, add documentation clarifying the distinction.
Issue 3: safe_outputs_config_helpers.go — 7 Near-Duplicate Config Generators
Severity: Medium–High
File: pkg/workflow/safe_outputs_config_helpers.go (368 lines, 16 private functions)
Seven consecutive functions (generateMax*Config) follow an identical structure — create a base map from resolveMaxForConfig, then conditionally add optional fields:
func generateMaxConfig(max *string, defaultMax int) map[string]any
func generateMaxWithAllowedLabelsConfig(max *string, defaultMax int, allowedLabels []string) map[string]any
func generateMaxWithTargetConfig(max *string, defaultMax int, target string) map[string]any
func generateMaxWithAllowedConfig(max *string, defaultMax int, allowed []string) map[string]any
func generateMaxWithAllowedAndBlockedConfig(max *string, defaultMax int, allowed []string, blocked []string) map[string]any
func generateMaxWithDiscussionFieldsConfig(max *string, defaultMax int, ...) map[string]any
func generateMaxWithReviewersConfig(max *string, defaultMax int, reviewers []string) map[string]anyEach function adds one or two fields to the base config. This pattern is highly repetitive and will grow as new safe-output types are added.
Recommendation: Consider a builder or options-struct pattern:
type MaxConfigOptions struct {
Allowed []string
Blocked []string
AllowedLabels []string
Reviewers []string
Target string
// etc.
}
func generateMaxConfig(max *string, defaultMax int, opts MaxConfigOptions) map[string]anyThis eliminates 6 near-duplicate functions and makes future extensions a single field addition.
Issue 4: update_entity_helpers.go — Oversized Multi-Concern File
Severity: Low–Medium
File: pkg/workflow/update_entity_helpers.go (543 lines, 8 functions)
This is the largest helper file in the package. It contains update logic for issues, pull requests, discussions, releases, and projects — four distinct entity domains in one file. This violates the "one file per feature" convention consistently applied elsewhere (e.g., create_issue.go, create_pull_request.go, create_discussion.go).
Recommendation: Split into domain-specific files following the existing create-entity pattern:
update_issue_helpers.goupdate_pull_request_helpers.goupdate_discussion_helpers.goupdate_release_helpers.go
Other Observations
String Utility Separation (Well-Designed)
pkg/stringutil/stringutil.go (general-purpose: Truncate, NormalizeWhitespace, ParseVersionValue, IsPositiveInteger) and pkg/workflow/strings.go (domain-specific: SanitizeName, ShortenCommand, GenerateHeredocDelimiter, PrettifyToolName, formatList) are properly separated. The workflow strings file explicitly documents the distinction. No action needed.
Validation File Architecture
The validation_helpers.go file explicitly documents that it consolidates 76+ duplicate validation patterns previously identified. The current architecture (generic helpers in validation_helpers.go, domain-specific validators in their own files) is correct. Domain-specific files like mcp_config_validation.go, safe_outputs_validation.go, etc., maintain appropriate separation.
CLI Helper Files
pkg/cli/helpers.go (56 lines, 3 functions), pkg/cli/pr_helpers.go (75 lines, 2 functions), and pkg/cli/deps_helpers.go (91 lines, 2 functions) are all appropriately sized and focused. No action needed.
Export Consistency Across Helper Files
Export patterns across helper files are inconsistent: engine_helpers.go exports ~87% of functions, config_helpers.go exports ~18%, validation_helpers.go exports 0%. While not urgent, a review pass to document intentional API boundaries would improve discoverability.
Recommendations Summary
| Priority | Finding | File(s) | Effort |
|---|---|---|---|
| Medium | Move safeUintToInt to map_helpers.go |
map_helpers.go, frontmatter_extraction_metadata.go |
< 1 hour |
| Medium | Document/unify parseIntValue vs ConvertToInt |
map_helpers.go |
< 1 hour |
| Medium | Replace 7 generateMax*Config functions with options struct |
safe_outputs_config_helpers.go |
2–4 hours |
| Low | Split update_entity_helpers.go by entity domain |
update_entity_helpers.go |
2–3 hours |
| Low | Review export patterns in helper files | Multiple | 1–2 hours |
Implementation Checklist
- Move
safeUintToIntwrapper tomap_helpers.go - Document (or unify)
parseIntValue/ConvertToIntdistinction - Refactor
generateMax*Configfunctions into options-struct pattern - Split
update_entity_helpers.gointo per-entity files - (Optional) Audit helper file export patterns for consistency
Analysis Metadata
- Total Go files analyzed: 597 (excluding
*_test.go) - Files with functions: 573
- Total functions cataloged: ~2,761
- Packages covered: 18 (
cli,workflow,parser,console,stringutil,fileutil, and 12 others) - Detection method: Naming pattern clustering, file-size analysis, Serena LSP semantic analysis
- Workflow run: §23384440781
- Analysis date: 2026-03-21
References:
Note
🔒 Integrity filter blocked 1 item
The following item were blocked because they don't meet the GitHub min-integrity level.
- #unknown
search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneSee Integrity Filtering for more information.
Generated by Semantic Function Refactoring · ◷
- expires on Mar 23, 2026, 5:11 PM UTC