-
Notifications
You must be signed in to change notification settings - Fork 46
Description
Description
The RunWorkflowTrials function in pkg/cli/trial_command.go is a 373-line god function with estimated cyclomatic complexity of 80+, making it untestable, unmaintainable, and error-prone.
Problem
Metrics:
- 373 lines (12x recommended maximum of 30 lines)
- 56 if statements (high branching complexity)
- 28 error checks (error handling fatigue)
- 24 for loops (nested iteration patterns)
- 5 defer closures (complex cleanup orchestration)
- Estimated Cyclomatic Complexity: 80+ (target: <10 per function)
Evidence:
# Function spans from line 196 to 568
$ sed -n '196,568p' ./pkg/cli/trial_command.go | wc -l
373
# Control flow density
$ awk 'NR >= 196 && NR <= 568' trial_command.go | grep "if err" | wc -l
28Responsibilities (violates SRP):
- Workflow spec parsing
- Repository mode determination (logical/clone/direct)
- Host repository creation and cleanup
- Secret management and deferred cleanup
- Workflow installation
- Trial execution orchestration (via nested anonymous function)
- Result collection and file I/O
Impact
- Maintenance burden: Any change requires understanding 373 lines of context
- Test coverage gaps: Cannot unit test individual phases
- Bug risk: Complex defer interactions and nested closures hard to reason about
- Reusability: Cannot reuse phases (e.g., "setup secrets") independently
Suggested Changes
Decompose into a phase-based pipeline following Command/Pipeline pattern:
// Phase-based orchestration
type TrialPhase interface {
Execute(ctx context.Context, state *TrialState) error
Rollback(ctx context.Context, state *TrialState) error
}
// Phases
type ParseSpecPhase struct{}
type SetupRepositoryPhase struct{}
type SetupSecretsPhase struct{}
type ExecuteTrialsPhase struct{}
type CollectResultsPhase struct{}
// Simplified orchestrator
func RunWorkflowTrials(config TrialConfig) error {
state := &TrialState{}
phases := []TrialPhase{
&ParseSpecPhase{},
&SetupRepositoryPhase{},
&SetupSecretsPhase{},
&ExecuteTrialsPhase{},
&CollectResultsPhase{},
}
// Execute with automatic rollback on failure
for i, phase := range phases {
if err := phase.Execute(ctx, state); err != nil {
// Rollback completed phases in reverse order
for j := i - 1; j >= 0; j-- {
phases[j].Rollback(ctx, state)
}
return err
}
}
return nil
}Benefits
- Cyclomatic complexity: 80+ → <10 per function
- Unit testable: Each phase can be tested independently
- Reusable: Phases can be composed for different trial workflows
- Clear error handling: Automatic rollback on failure
- Maintainable: Each phase has single responsibility
Files to Update
pkg/cli/trial_command.go:196-568- RefactorRunWorkflowTrialsfunction
New files to create:
pkg/cli/trial_phases.go- Phase implementationspkg/cli/trial_phases_test.go- Unit tests for each phase
Success Criteria
-
RunWorkflowTrialsfunction < 50 lines (orchestration only) - Each phase function < 100 lines, complexity < 15
- All phases have unit tests with >80% coverage
- Integration test validates full trial workflow
- All existing trial tests pass
-
make test-unitpasses -
make agent-finishpasses - No change in external API or CLI behavior
Priority
Critical - High complexity in critical workflow execution path creates maintenance and reliability risks
Estimated Effort: Large (8-12 hours including tests)
Source
Extracted from Sergo Report: Generic Type Opportunities + Method Complexity Analysis - Discussion #13246
Analysis Quote:
"373-line god function (
RunWorkflowTrials) with cyclomatic complexity ~80+"
Impact:
"This function is untestable in isolation and prone to bugs in edge cases"
References:
AI generated by Discussion Task Miner - Code Quality Improvement Agent
- expires on Feb 16, 2026, 1:28 PM UTC