Share Scriptorium run execution plumbing

internal/adapters/scriptorium/runner.go

@@ -140,61 +140,57 @@ func (r Runner) Render(ctx context.Context, req RenderRequest) (*RenderResult, e
 }
 
 func (r Runner) Run(ctx context.Context, req RunRequest) (*RunResult, error) {
-	if req.PromptID == "" {
+	result, err := r.executeRun(ctx, outputRunRequest{
-		return nil, fmt.Errorf("prompt id is required")
+		PromptID:        req.PromptID,
-	}
+		DataPackagePath: req.DataPackagePath,
-	if req.DataPackagePath == "" {
-		return nil, fmt.Errorf("data package path is required")
-	}
-	if req.OutputPath == "" {
-		return nil, fmt.Errorf("output path is required")
-	}
-	execution, err := r.execute(ctx, r.runArgs(req))
-	if err != nil {
-		return nil, fmt.Errorf("run scriptorium: %w", err)
-	}
-	result := &RunResult{
-		Command:         execution.argv(),
-		Stdout:          string(execution.result.Stdout),
-		Stderr:          string(execution.result.Stderr),
-		StdoutTruncated: execution.result.StdoutTruncated,
-		StderrTruncated: execution.result.StderrTruncated,
-		ExitCode:        execution.result.ExitCode,
 		OutputPath:      req.OutputPath,
+	}, "run scriptorium", "scriptorium run")
+	if err != nil {
+		if result == nil {
+			return nil, err
+		}
+		return result.runResult(), err
 	}
-	if execution.result.ExitCode != 0 {
+	return result.runResult(), nil
-		return result, fmt.Errorf("scriptorium run exited with code %d: %s", execution.result.ExitCode, result.Stderr)
-	}
-	return result, nil
 }
 
 func (r Runner) StructuredRun(ctx context.Context, req StructuredRunRequest) (*StructuredRunResult, error) {
-	if req.PromptID == "" {
+	result, err := r.executeRun(ctx, outputRunRequest{
-		return nil, fmt.Errorf("prompt id is required")
+		PromptID:        req.PromptID,
-	}
+		DataPackagePath: req.DataPackagePath,
-	if req.DataPackagePath == "" {
-		return nil, fmt.Errorf("data package path is required")
-	}
-	if req.OutputPath == "" {
-		return nil, fmt.Errorf("output path is required")
-	}
-	execution, err := r.execute(ctx, r.structuredRunArgs(req))
-	if err != nil {
-		return nil, fmt.Errorf("run scriptorium structured output: %w", err)
-	}
-	result := &StructuredRunResult{
-		Command:         execution.argv(),
-		Stdout:          string(execution.result.Stdout),
-		Stderr:          string(execution.result.Stderr),
-		StdoutTruncated: execution.result.StdoutTruncated,
-		StderrTruncated: execution.result.StderrTruncated,
-		ExitCode:        execution.result.ExitCode,
 		OutputPath:      req.OutputPath,
+	}, "run scriptorium structured output", "scriptorium structured run")
+	if err != nil {
+		if result == nil {
+			return nil, err
+		}
+		return result.structuredRunResult(), err
 	}
-	if execution.result.ExitCode != 0 {
+	return result.structuredRunResult(), nil
-		return result, fmt.Errorf("scriptorium structured run exited with code %d: %s", execution.result.ExitCode, result.Stderr)
+}
+
+func (result outputRunResult) runResult() *RunResult {
+	return &RunResult{
+		Command:         result.Command,
+		Stdout:          result.Stdout,
+		Stderr:          result.Stderr,
+		StdoutTruncated: result.StdoutTruncated,
+		StderrTruncated: result.StderrTruncated,
+		ExitCode:        result.ExitCode,
+		OutputPath:      result.OutputPath,
+	}
+}
+
+func (result outputRunResult) structuredRunResult() *StructuredRunResult {
+	return &StructuredRunResult{
+		Command:         result.Command,
+		Stdout:          result.Stdout,
+		Stderr:          result.Stderr,
+		StdoutTruncated: result.StdoutTruncated,
+		StderrTruncated: result.StderrTruncated,
+		ExitCode:        result.ExitCode,
+		OutputPath:      result.OutputPath,
 	}
-	return result, nil
 }
 
 type execution struct {
@@ -203,6 +199,55 @@ type execution struct {
 	result CommandResult
 }
 
+type outputRunRequest struct {
+	PromptID        string
+	DataPackagePath string
+	OutputPath      string
+}
+
+type outputRunResult struct {
+	Command         []string
+	Stdout          string
+	Stderr          string
+	StdoutTruncated bool
+	StderrTruncated bool
+	ExitCode        int
+	OutputPath      string
+}
+
+func (r Runner) executeRun(ctx context.Context, req outputRunRequest, executeContext string, exitContext string) (*outputRunResult, error) {
+	if req.PromptID == "" {
+		return nil, fmt.Errorf("prompt id is required")
+	}
+	if req.DataPackagePath == "" {
+		return nil, fmt.Errorf("data package path is required")
+	}
+	if req.OutputPath == "" {
+		return nil, fmt.Errorf("output path is required")
+	}
+	execution, err := r.execute(ctx, r.runArgs(RunRequest{
+		PromptID:        req.PromptID,
+		DataPackagePath: req.DataPackagePath,
+		OutputPath:      req.OutputPath,
+	}))
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", executeContext, err)
+	}
+	result := &outputRunResult{
+		Command:         execution.argv(),
+		Stdout:          string(execution.result.Stdout),
+		Stderr:          string(execution.result.Stderr),
+		StdoutTruncated: execution.result.StdoutTruncated,
+		StderrTruncated: execution.result.StderrTruncated,
+		ExitCode:        execution.result.ExitCode,
+		OutputPath:      req.OutputPath,
+	}
+	if execution.result.ExitCode != 0 {
+		return result, fmt.Errorf("%s exited with code %d: %s", exitContext, execution.result.ExitCode, result.Stderr)
+	}
+	return result, nil
+}
+
 func (r Runner) execute(ctx context.Context, args []string) (execution, error) {
 	binary := r.Binary
 	if binary == "" {
@@ -257,14 +302,6 @@ func (r Runner) runArgs(req RunRequest) []string {
 	return args
 }
 
-func (r Runner) structuredRunArgs(req StructuredRunRequest) []string {
-	return r.runArgs(RunRequest{
-		PromptID:        req.PromptID,
-		DataPackagePath: req.DataPackagePath,
-		OutputPath:      req.OutputPath,
-	})
-}
-
 type limitedBuffer struct {
 	data      []byte
 	limit     int

internal/adapters/scriptorium/runner_test.go

@@ -2,6 +2,7 @@ package scriptorium
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -236,53 +237,281 @@ func TestStructuredRunReturnsResultForNonzeroExit(t *testing.T) {
 	}
 }
 
-func TestStructuredRunValidatesRequiredFieldsBeforeExecution(t *testing.T) {
+func TestOutputRunsPreserveCapturedResultFields(t *testing.T) {
+	type commonResult struct {
+		Command         []string
+		Stdout          string
+		Stderr          string
+		StdoutTruncated bool
+		StderrTruncated bool
+		ExitCode        int
+		OutputPath      string
+	}
 	tests := []struct {
 		name string
-		req  StructuredRunRequest
+		run  func(Runner) (*commonResult, error)
-		want string
 	}{
 		{
-			name: "prompt id",
+			name: "Run",
-			req: StructuredRunRequest{
+			run: func(runner Runner) (*commonResult, error) {
-				DataPackagePath: "/tmp/hourly.data_package.yaml",
+				result, err := runner.Run(context.Background(), RunRequest{
-				OutputPath:      "/tmp/hourly.generated_text.raw.json",
+					PromptID:        "weather.markdown_report",
+					DataPackagePath: "/tmp/data_package.yaml",
+					OutputPath:      "/tmp/report.md",
+				})
+				if result == nil {
+					return nil, err
+				}
+				return &commonResult{
+					Command:         result.Command,
+					Stdout:          result.Stdout,
+					Stderr:          result.Stderr,
+					StdoutTruncated: result.StdoutTruncated,
+					StderrTruncated: result.StderrTruncated,
+					ExitCode:        result.ExitCode,
+					OutputPath:      result.OutputPath,
+				}, err
 			},
-			want: "prompt id is required",
 		},
 		{
-			name: "data package path",
+			name: "StructuredRun",
-			req: StructuredRunRequest{
+			run: func(runner Runner) (*commonResult, error) {
-				PromptID:   "weather.hourly_generated_text",
+				result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{
-				OutputPath: "/tmp/hourly.generated_text.raw.json",
+					PromptID:        "weather.markdown_report",
+					DataPackagePath: "/tmp/data_package.yaml",
+					OutputPath:      "/tmp/report.md",
+				})
+				if result == nil {
+					return nil, err
+				}
+				return &commonResult{
+					Command:         result.Command,
+					Stdout:          result.Stdout,
+					Stderr:          result.Stderr,
+					StdoutTruncated: result.StdoutTruncated,
+					StderrTruncated: result.StderrTruncated,
+					ExitCode:        result.ExitCode,
+					OutputPath:      result.OutputPath,
+				}, err
 			},
-			want: "data package path is required",
-		},
-		{
-			name: "output path",
-			req: StructuredRunRequest{
-				PromptID:        "weather.hourly_generated_text",
-				DataPackagePath: "/tmp/hourly.data_package.yaml",
-			},
-			want: "output path is required",
 		},
 	}
+
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			commands := &fakeCommands{}
+			commands := &fakeCommands{result: CommandResult{
-			runner := Runner{Commands: commands}
+				Stdout:          []byte("captured stdout"),
-			result, err := runner.StructuredRun(context.Background(), test.req)
+				Stderr:          []byte("captured stderr"),
+				StdoutTruncated: true,
+				StderrTruncated: true,
+			}}
+			runner := Runner{
+				Binary:     "/usr/local/bin/scriptorium",
+				ConfigPath: "/etc/scriptorium.yml",
+				Profile:    "weather",
+				Timeout:    15 * time.Second,
+				Commands:   commands,
+			}
+
+			result, err := test.run(runner)
+			if err != nil {
+				t.Fatalf("%s error = %v", test.name, err)
+			}
+			wantArgs := []string{
+				"run",
+				"--config", "/etc/scriptorium.yml",
+				"--profile", "weather",
+				"--prompt", "weather.markdown_report",
+				"--input", "data_package=/tmp/data_package.yaml",
+				"--out", "/tmp/report.md",
+			}
+			if !reflect.DeepEqual(commands.args, wantArgs) {
+				t.Fatalf("args = %#v, want %#v", commands.args, wantArgs)
+			}
+			if commands.timeout != 15*time.Second {
+				t.Fatalf("timeout = %s, want 15s", commands.timeout)
+			}
+			if !reflect.DeepEqual(result.Command, append([]string{"/usr/local/bin/scriptorium"}, wantArgs...)) {
+				t.Fatalf("Command = %#v, want full argv", result.Command)
+			}
+			if result.Stdout != "captured stdout" || result.Stderr != "captured stderr" {
+				t.Fatalf("captured output = %q/%q, want stdout/stderr", result.Stdout, result.Stderr)
+			}
+			if !result.StdoutTruncated || !result.StderrTruncated {
+				t.Fatalf("truncation flags = %t/%t, want both true", result.StdoutTruncated, result.StderrTruncated)
+			}
+			if result.ExitCode != 0 || result.OutputPath != "/tmp/report.md" {
+				t.Fatalf("result = %#v, want exit 0 and output path", result)
+			}
+		})
+	}
+}
+
+func TestOutputRunsReturnCapturedResultForNonzeroExit(t *testing.T) {
+	type commonResult struct {
+		Stdout          string
+		Stderr          string
+		StderrTruncated bool
+		ExitCode        int
+		OutputPath      string
+	}
+	tests := []struct {
+		name    string
+		run     func(Runner) (*commonResult, error)
+		wantErr string
+	}{
+		{
+			name: "Run",
+			run: func(runner Runner) (*commonResult, error) {
+				result, err := runner.Run(context.Background(), RunRequest{
+					PromptID:        "weather.markdown_report",
+					DataPackagePath: "/tmp/data_package.yaml",
+					OutputPath:      "/tmp/report.md",
+				})
+				if result == nil {
+					return nil, err
+				}
+				return &commonResult{
+					Stdout:          result.Stdout,
+					Stderr:          result.Stderr,
+					StderrTruncated: result.StderrTruncated,
+					ExitCode:        result.ExitCode,
+					OutputPath:      result.OutputPath,
+				}, err
+			},
+			wantErr: "scriptorium run exited with code 7: captured stderr",
+		},
+		{
+			name: "StructuredRun",
+			run: func(runner Runner) (*commonResult, error) {
+				result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{
+					PromptID:        "weather.markdown_report",
+					DataPackagePath: "/tmp/data_package.yaml",
+					OutputPath:      "/tmp/report.md",
+				})
+				if result == nil {
+					return nil, err
+				}
+				return &commonResult{
+					Stdout:          result.Stdout,
+					Stderr:          result.Stderr,
+					StderrTruncated: result.StderrTruncated,
+					ExitCode:        result.ExitCode,
+					OutputPath:      result.OutputPath,
+				}, err
+			},
+			wantErr: "scriptorium structured run exited with code 7: captured stderr",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			runner := Runner{
+				Commands: &fakeCommands{result: CommandResult{
+					Stdout:          []byte("captured stdout"),
+					Stderr:          []byte("captured stderr"),
+					StderrTruncated: true,
+					ExitCode:        7,
+				}},
+			}
+
+			result, err := test.run(runner)
 			if err == nil {
-				t.Fatal("StructuredRun() error = nil, want validation error")
+				t.Fatalf("%s error = nil, want nonzero exit error", test.name)
 			}
-			if result != nil {
+			if result == nil {
-				t.Fatalf("StructuredRun() result = %#v, want nil", result)
+				t.Fatalf("%s result = nil, want captured result", test.name)
 			}
-			if !strings.Contains(err.Error(), test.want) {
+			if err.Error() != test.wantErr {
-				t.Fatalf("StructuredRun() error = %v, want %q", err, test.want)
+				t.Fatalf("%s error = %q, want %q", test.name, err.Error(), test.wantErr)
 			}
-			if commands.calls != 0 {
+			if result.Stdout != "captured stdout" || result.Stderr != "captured stderr" || !result.StderrTruncated {
-				t.Fatalf("commands calls = %d, want no subprocess execution", commands.calls)
+				t.Fatalf("captured result = %#v, want stdout/stderr/truncation", result)
+			}
+			if result.ExitCode != 7 || result.OutputPath != "/tmp/report.md" {
+				t.Fatalf("result = %#v, want exit 7 and output path", result)
+			}
+		})
+	}
+}
+
+func TestOutputRunsValidateRequiredFieldsBeforeExecution(t *testing.T) {
+	tests := []struct {
+		name string
+		run  func(Runner, string, string, string) error
+	}{
+		{
+			name: "Run",
+			run: func(runner Runner, promptID string, dataPackagePath string, outputPath string) error {
+				result, err := runner.Run(context.Background(), RunRequest{
+					PromptID:        promptID,
+					DataPackagePath: dataPackagePath,
+					OutputPath:      outputPath,
+				})
+				if result != nil {
+					return fmt.Errorf("result = %#v, want nil", result)
+				}
+				return err
+			},
+		},
+		{
+			name: "StructuredRun",
+			run: func(runner Runner, promptID string, dataPackagePath string, outputPath string) error {
+				result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{
+					PromptID:        promptID,
+					DataPackagePath: dataPackagePath,
+					OutputPath:      outputPath,
+				})
+				if result != nil {
+					return fmt.Errorf("result = %#v, want nil", result)
+				}
+				return err
+			},
+		},
+	}
+	cases := []struct {
+		name            string
+		promptID        string
+		dataPackagePath string
+		outputPath      string
+		want            string
+	}{
+		{
+			name:            "prompt id",
+			dataPackagePath: "/tmp/data_package.yaml",
+			outputPath:      "/tmp/report.md",
+			want:            "prompt id is required",
+		},
+		{
+			name:       "data package path",
+			promptID:   "weather.markdown_report",
+			outputPath: "/tmp/report.md",
+			want:       "data package path is required",
+		},
+		{
+			name:            "output path",
+			promptID:        "weather.markdown_report",
+			dataPackagePath: "/tmp/data_package.yaml",
+			want:            "output path is required",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			for _, tc := range cases {
+				t.Run(tc.name, func(t *testing.T) {
+					commands := &fakeCommands{}
+					err := test.run(Runner{Commands: commands}, tc.promptID, tc.dataPackagePath, tc.outputPath)
+					if err == nil {
+						t.Fatalf("%s error = nil, want validation error", test.name)
+					}
+					if !strings.Contains(err.Error(), tc.want) {
+						t.Fatalf("%s error = %v, want %q", test.name, err, tc.want)
+					}
+					if commands.calls != 0 {
+						t.Fatalf("commands calls = %d, want no subprocess execution", commands.calls)
+					}
+				})
 			}
 		})
 	}