[object Object]

← back to Cli Printing Press

refactor(cli): simplify generate orchestration (#325)

545ffb94eefb98a17b002d226de708589c1e5a58 · 2026-04-26 19:54:38 -0700 · Trevin Chow

## Change: collapse duplicated generate command orchestration helpers

### Equivalence contract
- Inputs covered: generate --docs, generate --plan, and generate --spec paths that resolve output directories and apply generation flags.
- Ordering preserved: yes. Flag validation still happens before filesystem claims; dry-run still skips output claiming; spec traffic analysis still rejects page-context-only captures before generation.
- Tie-breaking: unchanged. Existing spec merge and HTTP transport strength tie-breaking remain in the same helper functions.
- Error semantics: same ExitError codes and message text for invalid --spec-source, --client-pattern, --transport, traffic-analysis loading, generation, validation, and output claiming errors.
- Laziness: unchanged; no eager spec parsing or generator work added.
- Short-circuit eval: unchanged; docs, plan, and spec branches still return independently.
- Floating-point: N/A.
- RNG / hash order: unchanged; map iteration in dry-run and generation untouched.
- Observable side-effects: intended identical stderr/stdout, output files, manifest writes, spec archive writes, cache behavior, and output directory claiming order.
- Type narrowing: N/A.
- Rerender behavior: N/A.

### Verification
- go test ./internal/cli
- go fmt ./...
- go test ./...
- go build -o ./printing-press ./cmd/printing-press
- go vet ./...
- golangci-lint run ./...
- scripts/golden.sh verify

No golden fixtures were updated.

LOC: internal/cli/root.go 1,100 -> 1,089 (-11).

Files touched

Diff

commit 545ffb94eefb98a17b002d226de708589c1e5a58
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun Apr 26 19:54:38 2026 -0700

    refactor(cli): simplify generate orchestration (#325)
    
    ## Change: collapse duplicated generate command orchestration helpers
    
    ### Equivalence contract
    - Inputs covered: generate --docs, generate --plan, and generate --spec paths that resolve output directories and apply generation flags.
    - Ordering preserved: yes. Flag validation still happens before filesystem claims; dry-run still skips output claiming; spec traffic analysis still rejects page-context-only captures before generation.
    - Tie-breaking: unchanged. Existing spec merge and HTTP transport strength tie-breaking remain in the same helper functions.
    - Error semantics: same ExitError codes and message text for invalid --spec-source, --client-pattern, --transport, traffic-analysis loading, generation, validation, and output claiming errors.
    - Laziness: unchanged; no eager spec parsing or generator work added.
    - Short-circuit eval: unchanged; docs, plan, and spec branches still return independently.
    - Floating-point: N/A.
    - RNG / hash order: unchanged; map iteration in dry-run and generation untouched.
    - Observable side-effects: intended identical stderr/stdout, output files, manifest writes, spec archive writes, cache behavior, and output directory claiming order.
    - Type narrowing: N/A.
    - Rerender behavior: N/A.
    
    ### Verification
    - go test ./internal/cli
    - go fmt ./...
    - go test ./...
    - go build -o ./printing-press ./cmd/printing-press
    - go vet ./...
    - golangci-lint run ./...
    - scripts/golden.sh verify
    
    No golden fixtures were updated.
    
    LOC: internal/cli/root.go 1,100 -> 1,089 (-11).
---
 internal/cli/root.go | 217 ++++++++++++++++++++++++---------------------------
 1 file changed, 103 insertions(+), 114 deletions(-)

diff --git a/internal/cli/root.go b/internal/cli/root.go
index 4b202b54..bf9836ee 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -136,65 +136,20 @@ func newGenerateCmd() *cobra.Command {
 				if err != nil {
 					return &ExitError{Code: ExitSpecError, Err: fmt.Errorf("parsing generated spec: %w", err)}
 				}
-				if specSource != "" {
-					normalized, err := normalizeSpecSource(specSource)
-					if err != nil {
-						return &ExitError{Code: ExitInputError, Err: err}
-					}
-					parsed.SpecSource = normalized
-				} else {
-					parsed.SpecSource = "docs"
-				}
-				if clientPattern != "" {
-					switch clientPattern {
-					case "rest", "proxy-envelope":
-						parsed.ClientPattern = clientPattern
-					default:
-						return &ExitError{Code: ExitInputError, Err: fmt.Errorf("--client-pattern must be one of: rest, proxy-envelope (got %q)", clientPattern)}
-					}
-				}
-				if httpTransport != "" {
-					normalized, err := normalizeHTTPTransport(httpTransport)
-					if err != nil {
-						return &ExitError{Code: ExitInputError, Err: err}
-					}
-					parsed.HTTPTransport = normalized
+				if err := applyGenerateSpecFlags(parsed, specSource, "docs", clientPattern, httpTransport); err != nil {
+					return err
 				}
 
-				explicitOutput := outputDir != ""
-				if outputDir == "" {
-					outputDir = pipeline.DefaultOutputDir(parsed.Name)
-				}
-				absOut, err := filepath.Abs(outputDir)
+				absOut, _, err := resolveGenerateOutputDir(outputDir, parsed.Name, force, true)
 				if err != nil {
-					return fmt.Errorf("resolving output path: %w", err)
-				}
-				absOut, err = claimOrForce(absOut, force, explicitOutput)
-				if err != nil {
-					return &ExitError{Code: ExitInputError, Err: err}
+					return err
 				}
 
-				enrichSpecFromCatalog(parsed)
-				gen := generator.New(parsed, absOut)
-				novelFeatures := loadResearchSources(gen, researchDir)
-				trafficAnalysis, err := loadTrafficAnalysisForGenerate(trafficAnalysisPath, nil, parsed.SpecSource)
+				novelFeatures, polished, err := runGenerateProject(parsed, absOut, generateProjectOptions{validate: validate, polish: polish, researchDir: researchDir, trafficAnalysisPath: trafficAnalysisPath})
 				if err != nil {
-					return &ExitError{Code: ExitInputError, Err: err}
-				}
-				applyHTTPTransportDefault(parsed, trafficAnalysis)
-				browsersniff.ApplyReachabilityDefaults(parsed, trafficAnalysis)
-				gen.TrafficAnalysis = trafficAnalysis
-				if err := gen.Generate(); err != nil {
-					return &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("generating project: %w", err)}
-				}
-				if validate {
-					if err := gen.Validate(); err != nil {
-						return &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("validating generated project: %w", err)}
-					}
+					return err
 				}
 
-				polished := runGeneratePolishPass(polish, parsed.Name, absOut)
-
 				if err := pipeline.WriteManifestForGenerate(pipeline.GenerateManifestParams{
 					APIName:       parsed.Name,
 					DocsURL:       docsURL,
@@ -243,17 +198,9 @@ func newGenerateCmd() *cobra.Command {
 					return &ExitError{Code: ExitInputError, Err: fmt.Errorf("plan contains no command definitions")}
 				}
 
-				explicitOutput := outputDir != ""
-				if outputDir == "" {
-					outputDir = pipeline.DefaultOutputDir(planSpec.CLIName)
-				}
-				absOut, err := filepath.Abs(outputDir)
+				absOut, _, err := resolveGenerateOutputDir(outputDir, planSpec.CLIName, force, true)
 				if err != nil {
-					return fmt.Errorf("resolving output path: %w", err)
-				}
-				absOut, err = claimOrForce(absOut, force, explicitOutput)
-				if err != nil {
-					return &ExitError{Code: ExitInputError, Err: err}
+					return err
 				}
 
 				if err := generator.GenerateFromPlan(planSpec, absOut); err != nil {
@@ -327,70 +274,23 @@ func newGenerateCmd() *cobra.Command {
 				apiSpec = mergeSpecs(specs, cliName)
 			}
 
-			if specSource != "" {
-				normalized, err := normalizeSpecSource(specSource)
-				if err != nil {
-					return &ExitError{Code: ExitInputError, Err: err}
-				}
-				apiSpec.SpecSource = normalized
-			}
-			if clientPattern != "" {
-				switch clientPattern {
-				case "rest", "proxy-envelope":
-					apiSpec.ClientPattern = clientPattern
-				default:
-					return &ExitError{Code: ExitInputError, Err: fmt.Errorf("--client-pattern must be one of: rest, proxy-envelope (got %q)", clientPattern)}
-				}
-			}
-			if httpTransport != "" {
-				normalized, err := normalizeHTTPTransport(httpTransport)
-				if err != nil {
-					return &ExitError{Code: ExitInputError, Err: err}
-				}
-				apiSpec.HTTPTransport = normalized
-			}
-
-			explicitOutput := outputDir != ""
-			if outputDir == "" {
-				outputDir = pipeline.DefaultOutputDir(apiSpec.Name)
+			if err := applyGenerateSpecFlags(apiSpec, specSource, "", clientPattern, httpTransport); err != nil {
+				return err
 			}
 
-			absOut, err := filepath.Abs(outputDir)
+			absOut, explicitOutput, err := resolveGenerateOutputDir(outputDir, apiSpec.Name, force, !dryRun)
 			if err != nil {
-				return fmt.Errorf("resolving output path: %w", err)
+				return err
 			}
 			if dryRun {
 				return printDryRun(apiSpec, absOut, specFiles)
 			}
-			absOut, err = claimOrForce(absOut, force, explicitOutput)
-			if err != nil {
-				return &ExitError{Code: ExitInputError, Err: err}
-			}
 
-			enrichSpecFromCatalog(apiSpec)
-			gen := generator.New(apiSpec, absOut)
-			novelFeatures := loadResearchSources(gen, researchDir)
-			trafficAnalysis, err := loadTrafficAnalysisForGenerate(trafficAnalysisPath, specFiles, apiSpec.SpecSource)
+			novelFeatures, polished, err := runGenerateProject(apiSpec, absOut, generateProjectOptions{validate: validate, polish: polish, researchDir: researchDir, trafficAnalysisPath: trafficAnalysisPath, specFiles: specFiles, rejectUnshippablePageContextTraffic: true})
 			if err != nil {
-				return &ExitError{Code: ExitInputError, Err: err}
-			}
-			if trafficAnalysisRequiresUnshippablePageContext(trafficAnalysis) {
-				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("traffic analysis says this target requires live browser page-context execution; persistent browser transport is not a shippable printed CLI runtime. Re-run discovery for a Surf/direct/browser-clearance replayable surface instead")}
-			}
-			applyHTTPTransportDefault(apiSpec, trafficAnalysis)
-			browsersniff.ApplyReachabilityDefaults(apiSpec, trafficAnalysis)
-			gen.TrafficAnalysis = trafficAnalysis
-			if err := gen.Generate(); err != nil {
-				return &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("generating project: %w", err)}
-			}
-			if validate {
-				if err := gen.Validate(); err != nil {
-					return &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("validating generated project: %w", err)}
-				}
+				return err
 			}
 
-			polished := runGeneratePolishPass(polish, apiSpec.Name, absOut)
-
 			// When --output was not explicitly supplied, normalize the output
 			// directory to the spec-derived name so default-path runs land in the
 			// expected slot (e.g., spec title "Cal.com" derives "cal-com-pp-cli").
@@ -496,6 +396,67 @@ func runGeneratePolishPass(enabled bool, apiName, outputDir string) bool {
 	return true
 }
 
+type generateProjectOptions struct {
+	validate                            bool
+	polish                              bool
+	researchDir                         string
+	trafficAnalysisPath                 string
+	specFiles                           []string
+	rejectUnshippablePageContextTraffic bool
+}
+
+func runGenerateProject(apiSpec *spec.APISpec, absOut string, opts generateProjectOptions) ([]pipeline.NovelFeatureManifest, bool, error) {
+	enrichSpecFromCatalog(apiSpec)
+	gen := generator.New(apiSpec, absOut)
+	novelFeatures := loadResearchSources(gen, opts.researchDir)
+	trafficAnalysis, err := loadTrafficAnalysisForGenerate(opts.trafficAnalysisPath, opts.specFiles, apiSpec.SpecSource)
+	if err != nil {
+		return nil, false, &ExitError{Code: ExitInputError, Err: err}
+	}
+	if opts.rejectUnshippablePageContextTraffic && trafficAnalysisRequiresUnshippablePageContext(trafficAnalysis) {
+		return nil, false, &ExitError{Code: ExitInputError, Err: fmt.Errorf("traffic analysis says this target requires live browser page-context execution; persistent browser transport is not a shippable printed CLI runtime. Re-run discovery for a Surf/direct/browser-clearance replayable surface instead")}
+	}
+	applyHTTPTransportDefault(apiSpec, trafficAnalysis)
+	browsersniff.ApplyReachabilityDefaults(apiSpec, trafficAnalysis)
+	gen.TrafficAnalysis = trafficAnalysis
+	if err := gen.Generate(); err != nil {
+		return nil, false, &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("generating project: %w", err)}
+	}
+	if opts.validate {
+		if err := gen.Validate(); err != nil {
+			return nil, false, &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("validating generated project: %w", err)}
+		}
+	}
+	return novelFeatures, runGeneratePolishPass(opts.polish, apiSpec.Name, absOut), nil
+}
+
+func applyGenerateSpecFlags(apiSpec *spec.APISpec, specSource, defaultSpecSource, clientPattern, httpTransport string) error {
+	if specSource != "" {
+		normalized, err := normalizeSpecSource(specSource)
+		if err != nil {
+			return &ExitError{Code: ExitInputError, Err: err}
+		}
+		apiSpec.SpecSource = normalized
+	} else if defaultSpecSource != "" {
+		apiSpec.SpecSource = defaultSpecSource
+	}
+	if clientPattern != "" {
+		normalized, err := normalizeClientPattern(clientPattern)
+		if err != nil {
+			return &ExitError{Code: ExitInputError, Err: err}
+		}
+		apiSpec.ClientPattern = normalized
+	}
+	if httpTransport != "" {
+		normalized, err := normalizeHTTPTransport(httpTransport)
+		if err != nil {
+			return &ExitError{Code: ExitInputError, Err: err}
+		}
+		apiSpec.HTTPTransport = normalized
+	}
+	return nil
+}
+
 func normalizeSpecSource(value string) (string, error) {
 	switch value {
 	case "", "official", "community", "sniffed", "docs":
@@ -507,6 +468,15 @@ func normalizeSpecSource(value string) (string, error) {
 	}
 }
 
+func normalizeClientPattern(value string) (string, error) {
+	switch value {
+	case "", "rest", "proxy-envelope":
+		return value, nil
+	default:
+		return "", fmt.Errorf("--client-pattern must be one of: rest, proxy-envelope (got %q)", value)
+	}
+}
+
 func normalizeHTTPTransport(value string) (string, error) {
 	switch value {
 	case "", spec.HTTPTransportStandard, spec.HTTPTransportBrowserChrome, spec.HTTPTransportBrowserChromeH3:
@@ -516,6 +486,25 @@ func normalizeHTTPTransport(value string) (string, error) {
 	}
 }
 
+func resolveGenerateOutputDir(outputDir, cliName string, force bool, claim bool) (string, bool, error) {
+	explicitOutput := outputDir != ""
+	if outputDir == "" {
+		outputDir = pipeline.DefaultOutputDir(cliName)
+	}
+	absOut, err := filepath.Abs(outputDir)
+	if err != nil {
+		return "", false, fmt.Errorf("resolving output path: %w", err)
+	}
+	if !claim {
+		return absOut, explicitOutput, nil
+	}
+	absOut, err = claimOrForce(absOut, force, explicitOutput)
+	if err != nil {
+		return "", false, &ExitError{Code: ExitInputError, Err: err}
+	}
+	return absOut, explicitOutput, nil
+}
+
 func applyHTTPTransportDefault(apiSpec *spec.APISpec, analysis *browsersniff.TrafficAnalysis) {
 	if apiSpec == nil || apiSpec.HTTPTransport != "" {
 		return

← 9b97201f docs(cli): use go fmt ./... for formatting instructions (#32  ·  back to Cli Printing Press  ·  refactor(cli): simplify scorecard dimension bookkeeping (#32 a2fe1581 →