-
Notifications
You must be signed in to change notification settings - Fork 46
Description
Description
The file pkg/workflow/compiler_safe_outputs_config.go (644 lines) contains highly repetitive code for configuring safe output handlers. Each of the 20+ handler types follows an identical pattern, resulting in approximately 400 lines of duplicated code (62% of the file).
This violates the DRY (Don't Repeat Yourself) principle and makes the code harder to maintain, test, and extend.
Current Pattern (Repeated 20+ Times)
// Pattern repeated for create_issue, add_comment, create_discussion, etc.
if data.SafeOutputs.HandlerX != nil {
cfg := data.SafeOutputs.HandlerX
handlerConfig := make(map[string]any)
if cfg.Max > 0 {
handlerConfig["max"] = cfg.Max
}
if cfg.TitlePrefix != "" {
handlerConfig["title_prefix"] = cfg.TitlePrefix
}
// ... more conditionals ...
config["handler_x"] = handlerConfig
}Problems
- Code Duplication: 400 lines of nearly identical code
- Maintenance Burden: Bug fixes must be applied 20+ times
- Extension Difficulty: Adding new handler types requires copying entire pattern
- Function Length:
addHandlerManagerConfigEnvVaris 532 lines (way over 50-line guideline) - Testing Overhead: Repetitive test cases needed for each handler
Suggested Refactoring
Option 1: Handler Registry Pattern
type HandlerConfigBuilder struct {
name string
config map[string]any
}
func (b *HandlerConfigBuilder) AddIfPositive(key string, value int) *HandlerConfigBuilder {
if value > 0 {
b.config[key] = value
}
return b
}
func (b *HandlerConfigBuilder) AddIfNotEmpty(key string, value string) *HandlerConfigBuilder {
if value != "" {
b.config[key] = value
}
return b
}
func (b *HandlerConfigBuilder) Build() map[string]any {
return b.config
}
// Registry of handler builders
var handlerRegistry = map[string]func(*SafeOutputsConfig) map[string]any{
"create_issue": func(cfg *SafeOutputsConfig) map[string]any {
return NewHandlerConfigBuilder("create_issue").
AddIfPositive("max", cfg.CreateIssue.Max).
AddIfNotEmpty("title_prefix", cfg.CreateIssue.TitlePrefix).
Build()
},
// ... other handlers ...
}
// Usage
func addHandlerManagerConfigEnvVar(data *WorkflowData) error {
config := make(map[string]any)
for name, builder := range handlerRegistry {
if handlerConfig := builder(data.SafeOutputs); len(handlerConfig) > 0 {
config[name] = handlerConfig
}
}
// ... rest of function ...
}Option 2: Reflection-Based Builder (More Dynamic)
type HandlerMetadata struct {
Name string
ConfigPath string // Path in SafeOutputsConfig struct
Fields []FieldMapping
}
type FieldMapping struct {
StructField string
ConfigKey string
Required bool
}
// Use reflection to generically build handler configs
func buildHandlerConfig(handler HandlerMetadata, safeOutputs *SafeOutputsConfig) map[string]any {
// Generic implementation using reflection
}Files Affected
pkg/workflow/compiler_safe_outputs_config.go(primary refactoring target)pkg/workflow/compiler_safe_outputs_config_test.go(may need test updates)- Possibly add new file:
pkg/workflow/handler_builder.go(for builder pattern)
Success Criteria
- File reduced from 644 lines to ~250 lines (60% reduction)
- Code duplication eliminated (400 lines saved)
- All existing tests pass without modification
- New handler types can be added with <10 lines of code
- Function length: no functions exceed 100 lines
- Code coverage maintained at current level (0.98 test-to-source ratio)
-
make testpasses all tests -
make lintreports no issues -
make recompilesuccessfully regenerates all workflows
Impact Analysis
Benefits:
- Maintainability: Bug fixes apply to all handlers automatically
- Extensibility: Adding new handlers requires minimal code
- Readability: Clear, concise code structure
- Testing: Simplified test cases
Risks:
- Refactoring complexity: Moderate (1 day of work estimated)
- Regression potential: Low (comprehensive test suite with 0.98 ratio)
Estimated Effort
1 day (8 hours)
- 3 hours: Implement builder pattern
- 2 hours: Refactor existing code
- 2 hours: Verify all tests pass and recompile workflows
- 1 hour: Code review and cleanup
Source
Extracted from Daily Compiler Code Quality Report #11729 - Analysis of compiler_safe_outputs_config.go (2026-01-25)
Priority
High - Significant code duplication (62% of file) impacts maintainability and makes bug fixes error-prone. Well-scoped refactoring with clear success criteria.
AI generated by Discussion Task Miner - Code Quality Improvement Agent
- expires on Feb 8, 2026, 2:04 PM UTC