Skip to content

refactor: split tools_types.go into type definitions and parsing logic#6919

Merged
pelikhan merged 5 commits intomainfrom
copilot/refactor-semantic-function-clustering
Dec 19, 2025
Merged

refactor: split tools_types.go into type definitions and parsing logic#6919
pelikhan merged 5 commits intomainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Contributor

Copilot AI commented Dec 19, 2025

Semantic Function Clustering Refactoring - Implementation Complete

This PR implements the beneficial refactoring from the semantic function clustering analysis.

Summary

Changes Implemented: Split tools_types.go for better file organization

✅ Completed Changes

  • Split tools_types.go
    • Created tools_parser.go for all parse* functions (450+ lines)
    • Kept tools_types.go for type definitions only
    • Fixed unused logger variable
    • All tests pass, linter clean
    • Impact: ⭐️ High - File names now accurately reflect contents

Quality Checks

  • Run make test-unit - All tests pass
  • Run make lint and make fmt - Clean
  • Verify no functional changes, only organization improvements

Files Changed

  • pkg/workflow/tools_types.go - Type definitions only
  • pkg/workflow/tools_parser.go - New file with parsing functions

Impact

Positive: Better file naming and separation of types vs parsing logic
No Breaking Changes: All functionality preserved

Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis - Code Organization Opportunities</issue_title>
<issue_description>This report presents a comprehensive semantic analysis of the Go codebase, identifying refactoring opportunities through function clustering, outlier detection, and duplicate analysis.

Executive Summary

Analysis Scope: 1,821 functions across 299 non-test Go files in 12 packages

Key Findings:

  • ✅ Strong naming conventions (get*, build*, parse*, generate*, validate*)
  • ⚠️ Functions scattered by type rather than grouped by purpose
  • ⚠️ Large monolithic files requiring splitting (js.go: 41 functions, scripts.go: 36 functions)
  • ⚠️ Flat CLI structure (99 files in single directory)
  • ⚠️ Generic duplicate patterns detected in helper functions

Estimated Impact:

  • Implementing top 4 recommendations: ~40% improvement in code navigability
  • Full refactoring: ~70% reduction in "time to find function"
  • Onboarding efficiency: New developers navigate ~3x faster
Package Distribution

Function Count by Package

Package Functions % of Total Priority
pkg/workflow/ 1,061 58.2% 🔴 HIGH
pkg/cli/ 516 28.3% 🟡 MEDIUM
pkg/parser/ 147 8.1% 🟢 LOW
pkg/console/ 30 1.6% ✅ Good
pkg/logger/ 19 1.0% ✅ Good
pkg/gitutil/ 13 0.7% ✅ Good
pkg/campaign/ 32 1.8% ✅ Good
Other packages 3 0.2% ✅ Good
Naming Pattern Analysis

Common Function Prefixes (pkg/workflow/)

Prefix Count Files Organization Status
get* 190 (18.1%) 78 🔴 Highly scattered
build* 112 (10.7%) 45 🟡 Partially scattered
parse* 101 (9.6%) 49 🔴 Highly scattered
generate* 99 (9.4%) 42 🟡 Partially scattered
validate* 52 (5.0%) 24 🟢 Some consolidation
extract* 35 (3.3%) 19 🟡 Scattered
format* 21 (2.0%) 11 🟢 Some consolidation
create* 19 (1.8%) 19 ✅ One per file (good!)

Insight: Functions with create* prefix follow the "one feature per file" pattern well, but most other patterns are scattered.

Critical Issues Identified

1. Outlier Functions (Functions in Wrong Files)

Issue #1A: tools_types.go Contains Parsers, Not Types

File: /home/runner/work/gh-aw/gh-aw/pkg/workflow/tools_types.go

Problem: 15 out of 19 functions (78%) are parse* functions, despite file name suggesting type definitions

Functions misplaced:

  • parseToolsFromFrontmatter()
  • parseMCPServersFromFrontmatter()
  • parseRuntimesFromFrontmatter()
  • parseSerenaConfig()
  • parseGitHubToolConfig()
  • parseGitHubToolset()
  • parseRemoteToolsConfig()
  • parseWebSearchConfig()
  • 7 more parsing functions...

Recommendation:

Split into:
- tools_types.go (type definitions only)
- tools_parser.go (all parse* functions)

Impact: High - Misleading file name causes confusion for developers


Issue #1B: Generic Helper Functions in close_entity_helpers.go and update_entity_helpers.go

Files:

  • /home/runner/work/gh-aw/gh-aw/pkg/workflow/close_entity_helpers.go:42 - parseCloseEntityConfig()
  • /home/runner/work/gh-aw/gh-aw/pkg/workflow/close_entity_helpers.go:91 - buildCloseEntityJob()
  • /home/runner/work/gh-aw/gh-aw/pkg/workflow/update_entity_helpers.go:52 - parseUpdateEntityConfig()
  • /home/runner/work/gh-aw/gh-aw/pkg/workflow/update_entity_helpers.go:85 - buildUpdateEntityJob()

Problem: These are generic template functions used by multiple entity-specific files, but named as if they're specific helpers

Detected Duplication Pattern: Both files implement nearly identical patterns:

  • Generic parseXXXEntityConfig() with entity type parameter
  • Generic buildXXXEntityJob() with entity type parameter
  • Both use the same ParseTargetConfig() from safe_output_builder.go
  • Both use the same buildSafeOutputJob() from safe_outputs_jobs.go

Code Similarity: ~85% similar structure between close and update helpers

Recommendation:

Consolidate into:
- entity_job_helpers.go (generic entity job building)
  OR
- safe_output_generic_builder.go (generic safe output patterns)

Impact: Medium-High - Reduces code duplication and clarifies these are generic patterns


2. Large Files Requiring Decomposition

Issue #2A: js.go - Mixed Responsibilities

File: /home/runner/work/gh-aw/gh-aw/pkg/workflow/js.go (41 functions)

Problem: File handles 3 distinct responsibilities:

  1. Comment Removal (10 functions)

    • removeBlockComments(), removeLineComments(), stripJSComments()
  2. YAML Formatting (15 functions)

    • formatForYAML(), needsYAMLQuoting(), `escapeBacks...

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 19, 2025 07:09
Separates type definitions from parsing logic for better organization.
- tools_types.go: keeps type definitions and helper methods
- tools_parser.go: contains all parse* functions and NewTools

Issue: #1A from semantic function clustering analysis

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
The toolsTypesLog variable is no longer needed after moving parsing
functions to tools_parser.go

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Documents analysis of semantic function clustering recommendations,
explaining which changes were beneficial and which were not needed.

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor Go code organization based on semantic analysis refactor: split tools_types.go into type definitions and parsing logic Dec 19, 2025
Copilot AI requested a review from mnkiefer December 19, 2025 07:25
@@ -0,0 +1,168 @@
# Semantic Function Clustering Refactoring - Analysis Results
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot remove file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 41421dc

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan December 19, 2025 12:55
@pelikhan pelikhan marked this pull request as ready for review December 19, 2025 13:36
@pelikhan pelikhan merged commit 13cd79f into main Dec 19, 2025
81 checks passed
@pelikhan pelikhan deleted the copilot/refactor-semantic-function-clustering branch December 19, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis - Code Organization Opportunities

3 participants