refactor: implement semantic function clustering improvements across pkg/

pkg/cli/codemod_permissions_read.go

@@ -0,0 +1,75 @@
+package cli
+
+import (
+	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+var permissionsReadCodemodLog = logger.New("cli:codemod_permissions_read")
+
+// getPermissionsReadCodemod creates a codemod for converting invalid "read" and "write" shorthands
+func getPermissionsReadCodemod() Codemod {
+	return Codemod{
+		ID:           "permissions-read-to-read-all",
+		Name:         "Convert invalid permissions shorthand",
+		Description:  "Converts 'permissions: read' to 'permissions: read-all' and 'permissions: write' to 'permissions: write-all' as per GitHub Actions spec",
+		IntroducedIn: "0.5.0",
+		Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
+			// Check if permissions exist
+			permissionsValue, hasPermissions := frontmatter["permissions"]
+			if !hasPermissions {
+				return content, false, nil
+			}
+
+			// Check if permissions uses invalid shorthand (read or write)
+			hasInvalidShorthand := false
+			if strValue, ok := permissionsValue.(string); ok {
+				if strValue == "read" || strValue == "write" {
+					hasInvalidShorthand = true
+				}
+			}
+
+			if !hasInvalidShorthand {
+				return content, false, nil
+			}
+
+			newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
+				var modified bool
+				result := make([]string, len(lines))
+				for i, line := range lines {
+					trimmedLine := strings.TrimSpace(line)
+
+					// Check for permissions line with shorthand
+					if strings.HasPrefix(trimmedLine, "permissions:") {
+						// Handle shorthand on same line: "permissions: read" or "permissions: write"
+						if strings.Contains(trimmedLine, ": read") && !strings.Contains(trimmedLine, "read-all") && !strings.Contains(trimmedLine, ": read\n") {
+							// Make sure it's "permissions: read" and not "contents: read"
+							if strings.TrimSpace(strings.Split(line, ":")[0]) == "permissions" {
+								result[i] = strings.Replace(line, ": read", ": read-all", 1)
+								modified = true
+								permissionsReadCodemodLog.Printf("Replaced 'permissions: read' with 'permissions: read-all' on line %d", i+1)
+								continue
+							}
+						} else if strings.Contains(trimmedLine, ": write") && !strings.Contains(trimmedLine, "write-all") {
+							// Make sure it's "permissions: write" and not "contents: write"
+							if strings.TrimSpace(strings.Split(line, ":")[0]) == "permissions" {
+								result[i] = strings.Replace(line, ": write", ": write-all", 1)
+								modified = true
+								permissionsReadCodemodLog.Printf("Replaced 'permissions: write' with 'permissions: write-all' on line %d", i+1)
+								continue
+							}
+						}
+					}
+
+					result[i] = line
+				}
+				return result, modified
+			})
+			if applied {
+				permissionsReadCodemodLog.Print("Applied permissions read/write to read-all/write-all migration")
+			}
+			return newContent, applied, err
+		},
+	}
+}

pkg/cli/codemod_permissions.go → pkg/cli/codemod_permissions_write.go

@@ -7,74 +7,6 @@ import (
 	"github.com/github/gh-aw/pkg/logger"
 )
 
-var permissionsReadCodemodLog = logger.New("cli:codemod_permissions_read")
-
-// getPermissionsReadCodemod creates a codemod for converting invalid "read" and "write" shorthands
-func getPermissionsReadCodemod() Codemod {
-	return Codemod{
-		ID:           "permissions-read-to-read-all",
-		Name:         "Convert invalid permissions shorthand",
-		Description:  "Converts 'permissions: read' to 'permissions: read-all' and 'permissions: write' to 'permissions: write-all' as per GitHub Actions spec",
-		IntroducedIn: "0.5.0",
-		Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
-			// Check if permissions exist
-			permissionsValue, hasPermissions := frontmatter["permissions"]
-			if !hasPermissions {
-				return content, false, nil
-			}
-
-			// Check if permissions uses invalid shorthand (read or write)
-			hasInvalidShorthand := false
-			if strValue, ok := permissionsValue.(string); ok {
-				if strValue == "read" || strValue == "write" {
-					hasInvalidShorthand = true
-				}
-			}
-
-			if !hasInvalidShorthand {
-				return content, false, nil
-			}
-
-			newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
-				var modified bool
-				result := make([]string, len(lines))
-				for i, line := range lines {
-					trimmedLine := strings.TrimSpace(line)
-
-					// Check for permissions line with shorthand
-					if strings.HasPrefix(trimmedLine, "permissions:") {
-						// Handle shorthand on same line: "permissions: read" or "permissions: write"
-						if strings.Contains(trimmedLine, ": read") && !strings.Contains(trimmedLine, "read-all") && !strings.Contains(trimmedLine, ": read\n") {
-							// Make sure it's "permissions: read" and not "contents: read"
-							if strings.TrimSpace(strings.Split(line, ":")[0]) == "permissions" {
-								result[i] = strings.Replace(line, ": read", ": read-all", 1)
-								modified = true
-								permissionsReadCodemodLog.Printf("Replaced 'permissions: read' with 'permissions: read-all' on line %d", i+1)
-								continue
-							}
-						} else if strings.Contains(trimmedLine, ": write") && !strings.Contains(trimmedLine, "write-all") {
-							// Make sure it's "permissions: write" and not "contents: write"
-							if strings.TrimSpace(strings.Split(line, ":")[0]) == "permissions" {
-								result[i] = strings.Replace(line, ": write", ": write-all", 1)
-								modified = true
-								permissionsReadCodemodLog.Printf("Replaced 'permissions: write' with 'permissions: write-all' on line %d", i+1)
-								continue
-							}
-						}
-					}
-
-					result[i] = line
-				}
-				return result, modified
-			})
-			if applied {
-				permissionsReadCodemodLog.Print("Applied permissions read/write to read-all/write-all migration")
-			}
-			return newContent, applied, err
-		},
-	}
-}
-
 var writePermissionsCodemodLog = logger.New("cli:codemod_permissions")
 
 // getWritePermissionsCodemod creates a codemod for converting write permissions to read

pkg/cli/trial_command.go

@@ -17,8 +17,8 @@ import (
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
-	"github.com/github/gh-aw/pkg/repoutil"
 	"github.com/github/gh-aw/pkg/sliceutil"
+	"github.com/github/gh-aw/pkg/stringutil"
 	"github.com/github/gh-aw/pkg/workflow"
 	"github.com/spf13/cobra"
 )
@@ -503,7 +503,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp
 			workflowResults = append(workflowResults, result)
 
 			// Save individual trial file
-			sanitizedTargetRepo := repoutil.SanitizeForFilename(targetRepoForFilename)
+			sanitizedTargetRepo := stringutil.SanitizeForFilename(targetRepoForFilename)
 			individualFilename := fmt.Sprintf("trials/%s-%s.%s.json", parsedSpec.WorkflowName, sanitizedTargetRepo, dateTimeID)
 			if err := saveTrialResult(individualFilename, result, opts.Verbose); err != nil {
 				fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save individual trial result: %v", err)))
@@ -537,7 +537,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp
 		if len(parsedSpecs) > 1 {
 			workflowNames := sliceutil.Map(parsedSpecs, func(spec *WorkflowSpec) string { return spec.WorkflowName })
 			workflowNamesStr := strings.Join(workflowNames, "-")
-			sanitizedTargetRepo := repoutil.SanitizeForFilename(targetRepoForFilename)
+			sanitizedTargetRepo := stringutil.SanitizeForFilename(targetRepoForFilename)
 			combinedFilename := fmt.Sprintf("trials/%s-%s.%s.json", workflowNamesStr, sanitizedTargetRepo, dateTimeID)
 			combinedResult := CombinedTrialResult{
 				WorkflowNames: workflowNames,
@@ -906,7 +906,7 @@ func copyTrialResultsToHostRepo(tempDir, dateTimeID string, workflowNames []stri
 	}
 
 	// Copy individual workflow result files
-	sanitizedTargetRepo := repoutil.SanitizeForFilename(targetRepoSlug)
+	sanitizedTargetRepo := stringutil.SanitizeForFilename(targetRepoSlug)
 	for _, workflowName := range workflowNames {
 		sourceFile := fmt.Sprintf("trials/%s-%s.%s.json", workflowName, sanitizedTargetRepo, dateTimeID)
 		destFile := filepath.Join(trialsDir, fmt.Sprintf("%s-%s.%s.json", workflowName, sanitizedTargetRepo, dateTimeID))

pkg/console/doc.go

@@ -0,0 +1,25 @@
+// Package console provides terminal UI components and formatting utilities for
+// the gh-aw CLI.
+//
+// # Naming Convention: Format* vs Render*
+//
+// Functions in this package follow a consistent naming convention:
+//
+//   - Format* functions return a formatted string for a single item or message.
+//     They are pure string transformations with no side effects.
+//     Examples: FormatSuccessMessage, FormatErrorMessage, FormatFileSize,
+//     FormatCommandMessage, FormatProgressMessage.
+//
+//   - Render* functions produce multi-element or structured output (tables, boxes,
+//     trees, structs). They may return strings, slices of strings, or write
+//     directly to output. They are used when the output requires layout or
+//     structural composition.
+//     Examples: RenderTable, RenderStruct, RenderTitleBox, RenderErrorBox,
+//     RenderInfoSection, RenderTree, RenderComposedSections.
+//
+// # Output Routing
+//
+// All diagnostic output (messages, warnings, errors) should be written to stderr.
+// Structured data output (JSON, hashes, graphs) should be written to stdout.
+// Use fmt.Fprintln(os.Stderr, ...) with the Format* helpers for diagnostic output.
+package console

pkg/constants/constants.go

@@ -21,6 +21,14 @@ const CLIExtensionPrefix CommandPrefix = "gh aw"
 //   - Easy refactoring: Can change implementation without affecting API
 //
 // See scratchpad/go-type-patterns.md for detailed guidance on type patterns.
+//
+// # Intentional Method Duplication
+//
+// Several string-based types below define identical String() and IsValid() method bodies.
+// This duplication is intentional: Go does not allow shared method sets for distinct named
+// types, so each type must define its own methods. The bodies are deliberately simple and
+// unlikely to diverge. Code-generation approaches (go:generate) were considered but add
+// more complexity than the duplication itself.
 
 // LineLength represents a line length in characters for expression formatting.
 // This semantic type distinguishes line lengths from arbitrary integers,

pkg/repoutil/repoutil.go

@@ -22,12 +22,3 @@ func SplitRepoSlug(slug string) (owner, repo string, err error) {
 	log.Printf("Split result: owner=%s, repo=%s", parts[0], parts[1])
 	return parts[0], parts[1], nil
 }
-
-// SanitizeForFilename converts a repository slug (owner/repo) to a filename-safe string.
-// Replaces "/" with "-". Returns "clone-mode" if the slug is empty.
-func SanitizeForFilename(slug string) string {
-	if slug == "" {
-		return "clone-mode"
-	}
-	return strings.ReplaceAll(slug, "/", "-")
-}

pkg/repoutil/repoutil_test.go

@@ -70,58 +70,13 @@ func TestSplitRepoSlug(t *testing.T) {
 	}
 }
 
-func TestSanitizeForFilename(t *testing.T) {
-	tests := []struct {
-		name     string
-		slug     string
-		expected string
-	}{
-		{
-			name:     "normal slug",
-			slug:     "github/gh-aw",
-			expected: "github-gh-aw",
-		},
-		{
-			name:     "empty slug",
-			slug:     "",
-			expected: "clone-mode",
-		},
-		{
-			name:     "slug with multiple slashes",
-			slug:     "owner/repo/extra",
-			expected: "owner-repo-extra",
-		},
-		{
-			name:     "slug with hyphen",
-			slug:     "owner/my-repo",
-			expected: "owner-my-repo",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := SanitizeForFilename(tt.slug)
-			if result != tt.expected {
-				t.Errorf("SanitizeForFilename(%q) = %q; want %q", tt.slug, result, tt.expected)
-			}
-		})
-	}
-}
-
 func BenchmarkSplitRepoSlug(b *testing.B) {
 	slug := "github/gh-aw"
 	for b.Loop() {
 		_, _, _ = SplitRepoSlug(slug)
 	}
 }
 
-func BenchmarkSanitizeForFilename(b *testing.B) {
-	slug := "github/gh-aw"
-	for b.Loop() {
-		_ = SanitizeForFilename(slug)
-	}
-}
-
 // Additional edge case tests
 
 func TestSplitRepoSlug_Whitespace(t *testing.T) {
@@ -230,49 +185,6 @@ func TestSplitRepoSlug_SpecialCharacters(t *testing.T) {
 	}
 }
 
-func TestSanitizeForFilename_SpecialCases(t *testing.T) {
-	tests := []struct {
-		name     string
-		slug     string
-		expected string
-	}{
-		{
-			name:     "multiple slashes",
-			slug:     "owner/repo/extra",
-			expected: "owner-repo-extra",
-		},
-		{
-			name:     "leading slash",
-			slug:     "/owner/repo",
-			expected: "-owner-repo",
-		},
-		{
-			name:     "trailing slash",
-			slug:     "owner/repo/",
-			expected: "owner-repo-",
-		},
-		{
-			name:     "only slashes",
-			slug:     "///",
-			expected: "---",
-		},
-		{
-			name:     "single character owner and repo",
-			slug:     "a/b",
-			expected: "a-b",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := SanitizeForFilename(tt.slug)
-			if result != tt.expected {
-				t.Errorf("SanitizeForFilename(%q) = %q; want %q", tt.slug, result, tt.expected)
-			}
-		})
-	}
-}
-
 func TestSplitRepoSlug_Idempotent(t *testing.T) {
 	// Test that splitting and rejoining gives the same result
 	slugs := []string{

pkg/stringutil/sanitize.go

@@ -179,3 +179,17 @@ func SanitizeToolID(toolID string) string {
 
 	return cleaned
 }
+
+// SanitizeForFilename converts a repository slug (owner/repo) to a filename-safe string.
+// Replaces "/" with "-". Returns "clone-mode" if the slug is empty.
+//
+// Examples:
+//
+//	SanitizeForFilename("owner/repo")  // returns "owner-repo"
+//	SanitizeForFilename("")            // returns "clone-mode"
+func SanitizeForFilename(slug string) string {
+	if slug == "" {
+		return "clone-mode"
+	}
+	return strings.ReplaceAll(slug, "/", "-")
+}