refactor(ui,core,cmd): decouple terminal presentation from execution engine

fix: correct milestone denominator drift and step numbering (4 → 8)

- Introduce Reporter interface in internal/types

- Implement SpinnerReporter (TTY) and PlainReporter (CI/dry-run)

- Add factory with character-device detection for automatic UI selection

- Remove StepLogger and goroutine-based spinner management from core

- Synchronize milestone denominators across cmd and core

- Update unit tests to use Reporter abstraction

Author: Test User

cmd/apply.go

@@ -7,10 +7,15 @@ import (
 	"fmt"
 	"path/filepath"
 	"scbake/internal/core"
+	"scbake/internal/ui"
 
 	"github.com/spf13/cobra"
 )
 
+const (
+	runApplyTotalSteps = 5 // Now 5 steps, as git steps are part of template logic
+)
+
 var (
 	langFlag string
 	withFlag []string
@@ -44,7 +49,10 @@ var applyCmd = &cobra.Command{
 			Force:           force,           // force is the global flag.
 		}
 
-		if err := core.RunApply(rc); err != nil {
+		// Initialize modular UI reporter using the factory
+		reporter := ui.NewReporter(runApplyTotalSteps, dryRun)
+
+		if err := core.RunApply(rc, reporter); err != nil {
 			return err
 		}
 		fmt.Println("✅ Success! 'apply' command finished.")

cmd/new.go

@@ -7,13 +7,18 @@ import (
 	"fmt"
 	"os"
 	"scbake/internal/core"
+	"scbake/internal/ui"
 	"scbake/internal/util/fileutil"
 
 	"github.com/spf13/cobra"
 )
 
-// Steps in the new command run
-const newCmdTotalSteps = 4
+// newCmdTotalSteps represents the total milestones in the 'new' workflow:
+// 1. Directory creation
+// 2. Template application start
+// 3-7. Internal RunApply milestones (Load, Plan, Execute, Manifest, Commit)
+// 8. Finalization
+const newCmdTotalSteps = 8
 
 var (
 	newLangFlag string
@@ -27,13 +32,10 @@ var newCmd = &cobra.Command{
 	Args:  cobra.ExactArgs(1),
 	RunE: func(_ *cobra.Command, args []string) error {
 		projectName := args[0]
-
-		// Flag to track directory creation
 		dirCreated := false
 
-		// runNew takes a pointer to dirCreated to track creation status
 		if err := runNew(projectName, &dirCreated); err != nil {
-			// SAFETY CHECK: Only clean up the directory if we created it during this command.
+			// Cleanup: Only remove the directory if it was created during this session.
 			if dirCreated {
 				fmt.Fprintf(os.Stderr, "Cleaning up failed project directory '%s'...\n", projectName)
 				_ = os.RemoveAll(projectName)
@@ -46,31 +48,31 @@ var newCmd = &cobra.Command{
 	},
 }
 
-// runNew takes a pointer to dirCreated to track creation status.
+// runNew coordinates the project directory setup and delegates template application to the core engine.
 func runNew(projectName string, dirCreated *bool) error {
-	logger := core.NewStepLogger(newCmdTotalSteps, dryRun)
+	reporter := ui.NewReporter(newCmdTotalSteps, dryRun)
 
 	// Capture original working directory before any changes.
 	cwd, err := os.Getwd()
 	if err != nil {
 		return fmt.Errorf("failed to get cwd: %w", err)
 	}
 
-	// 1. Check if directory exists
+	// Verify target directory availability
 	if _, err := os.Stat(projectName); !os.IsNotExist(err) {
 		return fmt.Errorf("directory '%s' already exists", projectName)
 	}
 
-	// 2. Create directory
-	logger.Log("📁", "Creating directory: "+projectName)
+	// Initialize project directory
+	reporter.Step("📁", "Creating directory: "+projectName)
 	if !dryRun {
 		if err := os.Mkdir(projectName, fileutil.DirPerms); err != nil {
 			return err
 		}
-		*dirCreated = true // Set flag: successfully created directory
+		*dirCreated = true
 	}
 
-	// 3. CD into directory
+	// Relocate to target directory for atomic scaffolding
 	if !dryRun {
 		if err := os.Chdir(projectName); err != nil {
 			return err
@@ -82,32 +84,30 @@ func runNew(projectName string, dirCreated *bool) error {
 		}()
 	}
 
-	// Bootstrap manifest so the engine can find the project root
+	// Bootstrap manifest for project root discovery
 	if !dryRun {
 		if err := os.WriteFile(fileutil.ManifestFileName, []byte(""), fileutil.PrivateFilePerms); err != nil {
 			return fmt.Errorf("failed to bootstrap manifest: %w", err)
 		}
 	}
 
-	// 6. Run the 'apply' logic
-	logger.Log("🚀", "Applying templates...")
+	// Delegate template and language pack application to the core executor
+	reporter.Step("🚀", "Applying templates...")
 	rc := core.RunContext{
 		LangFlag:        newLangFlag,
 		WithFlag:        newWithFlag,
-		TargetPath:      ".",    // Now correctly relative to the new directory
-		DryRun:          dryRun, // Use global flag
-		Force:           force,  // Use global flag
+		TargetPath:      ".",
+		DryRun:          dryRun,
+		Force:           force,
 		ManifestPathArg: ".",
 	}
 
-	// RunApply prints its own logs.
-	if err := core.RunApply(rc); err != nil {
+	if err := core.RunApply(rc, reporter); err != nil {
 		return err
 	}
 
-	// Update the total steps for the logger, using the exported method.
-	logger.SetTotalSteps(newCmdTotalSteps)
-	logger.Log("✨", "Finalizing project...")
+	// Finalize output
+	reporter.Step("✨", "Finalizing project...")
 	return nil
 }
 

internal/core/executor.go

@@ -7,62 +7,27 @@ import (
 	"fmt"
 	"scbake/internal/types"
 	"sort"
-	"time"
 )
 
-// Braille spinner characters
-var spinnerChars = []string{"⣷", "⣯", "⣟", "⡿", "⢿", "⣻", "⣽", "⣾"}
-
-// spinnerDelay defines the interval for spinner character updates.
-const spinnerDelay = 100 * time.Millisecond
-
-// Execute runs the plan.
-func Execute(plan *types.Plan, tc types.TaskContext) error {
-	// Sort tasks by priority
+// Execute runs the plan and reports progress via the provided Reporter.
+func Execute(plan *types.Plan, tc types.TaskContext, reporter types.Reporter) error {
 	sort.SliceStable(plan.Tasks, func(i, j int) bool {
 		return plan.Tasks[i].Priority() < plan.Tasks[j].Priority()
 	})
 
 	for i, task := range plan.Tasks {
-		// If we're in a dry run, just print the description
-		if tc.DryRun {
-			fmt.Printf("  [DRY RUN] %s\n", task.Description())
-			continue
-		}
+		reporter.TaskStart(task.Description(), i+1, len(plan.Tasks))
 
-		// Spinner Logic
-		done := make(chan struct{})
-		go func() {
-			j := 0
-			for {
-				select {
-				case <-done:
-					return
-				default:
-					// Print the spinner, prefix, and description
-					prefix := fmt.Sprintf("[%d/%d]", i+1, len(plan.Tasks))
-					line := fmt.Sprintf("\r%s %s %s", prefix, spinnerChars[j%len(spinnerChars)], task.Description())
-					fmt.Print(line)
-					j++
-					time.Sleep(spinnerDelay)
-				}
-			}
-		}()
-
-		err := task.Execute(tc) // Run the actual task
+		var err error
+		if !tc.DryRun {
+			err = task.Execute(tc)
+		}
 
-		close(done) // Stop the spinner goroutine
+		reporter.TaskEnd(err)
 
-		// Print the final line
-		prefix := fmt.Sprintf("[%d/%d]", i+1, len(plan.Tasks))
 		if err != nil {
-			// Print a clear error message, overwriting the spinner line
-			fmt.Printf("\r%s ❌ %s\n", prefix, task.Description())
 			return fmt.Errorf("task failed (%s): %w", task.Description(), err)
 		}
-
-		// Print a clear success message, overwriting the spinner line
-		fmt.Printf("\r%s ✅ %s\n", prefix, task.Description())
 	}
 
 	return nil

internal/core/run.go

@@ -19,44 +19,12 @@ import (
 	"scbake/pkg/templates"
 )
 
-// Define constants for step logging and cyclomatic complexity reduction
-const (
-	runApplyTotalSteps  = 5 // Now 5 steps, as git steps are part of template logic
-	langApplyTotalSteps = 5
-)
-
-// StepLogger helps print consistent step messages
-type StepLogger struct {
-	currentStep int
-	totalSteps  int // Keep unexported
-	DryRun      bool
-}
-
-// NewStepLogger creates a new StepLogger instance.
-func NewStepLogger(totalSteps int, dryRun bool) *StepLogger {
-	return &StepLogger{totalSteps: totalSteps, DryRun: dryRun}
-}
-
-// Log prints the current step message.
-func (l *StepLogger) Log(emoji, message string) {
-	l.currentStep++
-	if l.DryRun && l.currentStep > 2 {
-		return
-	}
-	fmt.Printf("[%d/%d] %s %s\n", l.currentStep, l.totalSteps, emoji, message)
-}
-
-// SetTotalSteps updates the total number of steps for logging.
-func (l *StepLogger) SetTotalSteps(newTotal int) {
-	l.totalSteps = newTotal
-}
-
-// RunContext holds all the flags and args for a run.
+// RunContext holds the flags and arguments for a single execution run.
 type RunContext struct {
 	LangFlag        string
 	WithFlag        []string
-	TargetPath      string // Used for execution (absolute path)
-	ManifestPathArg string // Used for manifest/template rendering (relative path)
+	TargetPath      string // Absolute path for execution stability.
+	ManifestPathArg string // Relative path for manifest portability.
 	DryRun          bool
 	Force           bool
 }
@@ -67,11 +35,9 @@ type manifestChanges struct {
 	Templates []types.Template
 }
 
-// RunApply is the main logic for the 'apply' command, extracted.
-func RunApply(rc RunContext) error {
-	logger := NewStepLogger(runApplyTotalSteps, rc.DryRun)
-
-	logger.Log("📖", "Loading manifest ("+fileutil.ManifestFileName+")...")
+// RunApply orchestrates the template application process.
+func RunApply(rc RunContext, reporter types.Reporter) error {
+	reporter.Step("📖", "Loading manifest ("+fileutil.ManifestFileName+")...")
 
 	// 1. Root Discovery & Manifest Load
 	m, rootPath, err := manifest.Load(rc.TargetPath)
@@ -101,7 +67,7 @@ func RunApply(rc RunContext) error {
 		}()
 	}
 
-	logger.Log("📝", "Building execution plan...")
+	reporter.Step("📝", "Building execution plan...")
 
 	// Deduplicate requested templates to ensure idempotency
 	rc.WithFlag = deduplicateTemplates(rc.WithFlag)
@@ -127,34 +93,27 @@ func RunApply(rc RunContext) error {
 	}
 
 	if rc.DryRun {
-		fmt.Println("DRY RUN: No changes will be made.")
-		fmt.Println("Plan contains the following tasks:")
-		return Execute(plan, tc)
+		return Execute(plan, tc, reporter)
 	}
 
-	// 3. Execute and Finalize
-	// We pass the transaction and paths down.
-	return executeAndFinalize(logger, plan, tc, m, changes, rootPath, tx)
+	return executeAndFinalize(reporter, plan, tc, m, changes, rootPath, tx)
 }
 
-// executeAndFinalize runs the plan, updates manifest, and commits the transaction.
 func executeAndFinalize(
-	logger *StepLogger,
+	reporter types.Reporter,
 	plan *types.Plan,
 	tc types.TaskContext,
 	m *types.Manifest,
 	changes *manifestChanges,
 	rootPath string,
 	tx *transaction.Manager,
 ) error {
-	logger.Log("🚀", "Executing plan...")
-
-	// Run all tasks. They will auto-track changes via tc.Tx.
-	if err := Execute(plan, tc); err != nil {
-		return fmt.Errorf("task execution failed: %w", err)
+	reporter.Step("🚀", "Executing plan...")
+	if err := Execute(plan, tc, reporter); err != nil {
+		return err
 	}
 
-	logger.Log("✍️", "Updating manifest...")
+	reporter.Step("✍️", "Updating manifest...")
 	updateManifest(m, changes)
 
 	// We track the manifest file itself before saving.
@@ -169,7 +128,7 @@ func executeAndFinalize(
 		return fmt.Errorf("manifest save failed: %w", err)
 	}
 
-	logger.Log("✅", "Committing transaction...")
+	reporter.Step("✅", "Committing transaction...")
 	// Point of No Return: We delete the backups.
 	if err := tx.Commit(); err != nil {
 		return fmt.Errorf("failed to commit transaction: %w", err)
@@ -221,7 +180,7 @@ func buildPlan(rc RunContext) (*types.Plan, string, *manifestChanges, error) {
 	plan := &types.Plan{Tasks: []types.Task{}}
 	changes := &manifestChanges{}
 	commitMessage := "scbake: Apply templates"
-	didSomething := false // Flag to ensure at least one action is requested
+	didSomething := false
 
 	if rc.LangFlag != "" {
 		didSomething = true
@@ -245,7 +204,7 @@ func buildPlan(rc RunContext) (*types.Plan, string, *manifestChanges, error) {
 
 	// Only fail if neither a language nor tooling was specified.
 	if !didSomething {
-		return nil, "", nil, errors.New("no language or templates specified. Use --lang or --with")
+		return nil, "", nil, errors.New("no language or templates specified")
 	}
 
 	return plan, commitMessage, changes, nil
@@ -281,7 +240,6 @@ func handleLangFlag(rc RunContext, plan *types.Plan, changes *manifestChanges) (
 
 	plan.Tasks = append(plan.Tasks, langTasks...)
 
-	// Sanitize the project name for the manifest
 	projectName, err := util.SanitizeModuleName(rc.ManifestPathArg)
 	if err != nil {
 		return "", fmt.Errorf("could not determine project name: %w", err)

internal/core/run_test.go

@@ -11,6 +11,7 @@ import (
 	"scbake/internal/filesystem/transaction"
 	"scbake/internal/manifest"
 	"scbake/internal/types"
+	"scbake/internal/ui"
 	"scbake/internal/util/fileutil"
 	"testing"
 )
@@ -94,9 +95,11 @@ func TestExecuteAndFinalize_Rollback(t *testing.T) {
 	}
 
 	// Run logic
-	logger := NewStepLogger(2, false)
+	// executeAndFinalize calls Step() 3 times (Execute, Update Manifest, Commit).
+	// We use 3 as the denominator to ensure the UI doesn't drift.
+	reporter := ui.NewPlainReporter(3, false)
 	// Execute should fail
-	err = executeAndFinalize(logger, plan, tc, m, changes, rootPath, tx)
+	err = executeAndFinalize(reporter, plan, tc, m, changes, rootPath, tx)
 
 	// Assert Failure
 	if err == nil {
@@ -162,8 +165,9 @@ func TestExecuteAndFinalize_Success(t *testing.T) {
 		Tx:         tx,
 	}
 
-	logger := NewStepLogger(1, false)
-	if err := executeAndFinalize(logger, plan, tc, m, changes, rootPath, tx); err != nil {
+	// executeAndFinalize calls Step() exactly 3 times.
+	reporter := ui.NewPlainReporter(3, false)
+	if err := executeAndFinalize(reporter, plan, tc, m, changes, rootPath, tx); err != nil {
 		t.Fatalf("Execution failed: %v", err)
 	}