diff --git a/internal/adapter/cli/run.go b/internal/adapter/cli/run.go index 471f608..00af7af 100644 --- a/internal/adapter/cli/run.go +++ b/internal/adapter/cli/run.go @@ -31,6 +31,7 @@ const ( ) type runConfig struct { + promptDir string profileDir string promptID string profileID string @@ -42,6 +43,7 @@ type runConfig struct { model string temperature float64 maxTokens int + topP float64 schemaDir string timeout time.Duration @@ -50,10 +52,13 @@ type runConfig struct { modelSet bool temperatureSet bool maxTokensSet bool + topPSet bool + timeoutSet bool } type serveConfig struct { addr string + promptDir string profileDir string schemaDir string llmBaseURL string @@ -127,7 +132,7 @@ func runCommand(args []string, stdout, stderr io.Writer) int { } runner := usecase.NewRunner( - promptdef.NewFilesystemRepository(cfg.profileDir), + promptdef.NewFilesystemRepository(cfg.promptDir), profile.NewFilesystemRepository(cfg.profileDir), artifactadapter.NewCompositeReader(), prompt.NewGoRenderer(), @@ -136,14 +141,17 @@ func runCommand(args []string, stdout, stderr io.Writer) int { ) var modelOverride *domain.ExecutionTarget - if cfg.llmBaseURLSet || cfg.modelSet || cfg.temperatureSet || cfg.maxTokensSet || cfg.apiKeyEnvSet { + if cfg.llmBaseURLSet || cfg.modelSet || cfg.temperatureSet || cfg.maxTokensSet || cfg.topPSet || cfg.apiKeyEnvSet || cfg.timeoutSet { modelOverride = &domain.ExecutionTarget{ - Endpoint: cfg.llmBaseURL, - Model: cfg.model, - Temperature: cfg.temperature, - MaxTokens: cfg.maxTokens, - TimeoutSeconds: int(cfg.timeout.Seconds()), - APIKeyEnv: cfg.apiKeyEnv, + Endpoint: cfg.llmBaseURL, + Model: cfg.model, + Temperature: cfg.temperature, + MaxTokens: cfg.maxTokens, + TopP: cfg.topP, + APIKeyEnv: cfg.apiKeyEnv, + } + if cfg.timeoutSet { + modelOverride.TimeoutSeconds = int(cfg.timeout.Seconds()) } } @@ -186,7 +194,7 @@ func serveCommand(args []string, stderr io.Writer) int { } runner := usecase.NewRunner( - promptdef.NewFilesystemRepository(cfg.profileDir), + promptdef.NewFilesystemRepository(cfg.promptDir), profile.NewFilesystemRepository(cfg.profileDir), artifactadapter.NewCompositeReader(), prompt.NewGoRenderer(), @@ -214,9 +222,10 @@ func parseRunArgs(args []string) (*runConfig, error) { fs := flag.NewFlagSet("run", flag.ContinueOnError) fs.SetOutput(io.Discard) - fs.StringVar(&cfg.profileDir, "profile-dir", "", "directory containing prompt definition YAML files") - fs.StringVar(&cfg.promptID, "prompt-id", "", "prompt ID to run") - fs.StringVar(&cfg.profileID, "profile-id", "", "optional execution profile ID; if omitted, prompt default_profile is used") + fs.StringVar(&cfg.promptDir, "prompt-dir", "", "directory containing prompt definition YAML files") + fs.StringVar(&cfg.profileDir, "profile-dir", "", "directory containing execution profile YAML files") + fs.StringVar(&cfg.promptID, "prompt", "", "prompt ID to run") + fs.StringVar(&cfg.profileID, "profile", "", "optional execution profile ID; if omitted, prompt default_profile is used") fs.Var(&cfg.inputRaw, "input", "input mapping(s): name=path (repeatable, comma-separated)") fs.Var(&cfg.varRaw, "var", "variable mapping(s): name=value (repeatable, comma-separated)") fs.StringVar(&cfg.outputPath, "out", "", "optional output file path") @@ -225,8 +234,11 @@ func parseRunArgs(args []string) (*runConfig, error) { fs.StringVar(&cfg.model, "model", "", "model name") fs.Float64Var(&cfg.temperature, "temperature", 0, "optional temperature override") fs.IntVar(&cfg.maxTokens, "max-tokens", 0, "optional max tokens override") + fs.Float64Var(&cfg.topP, "top-p", 0, "optional top_p override") fs.StringVar(&cfg.schemaDir, "schema-dir", defaults.SchemaDirDefault, "base directory for validation schemas") fs.DurationVar(&cfg.timeout, "timeout", defaults.LLMRequestTimeoutDefault, "LLM request timeout") + fs.StringVar(&cfg.promptID, "prompt-id", "", "deprecated alias for --prompt") + fs.StringVar(&cfg.profileID, "profile-id", "", "deprecated alias for --profile") if err := fs.Parse(args); err != nil { return nil, err @@ -235,15 +247,19 @@ func parseRunArgs(args []string) (*runConfig, error) { return nil, fmt.Errorf("unexpected positional args: %v", fs.Args()) } + if strings.TrimSpace(cfg.promptDir) == "" { + return nil, errors.New("--prompt-dir is required") + } if strings.TrimSpace(cfg.profileDir) == "" { return nil, errors.New("--profile-dir is required") } if strings.TrimSpace(cfg.promptID) == "" { - return nil, errors.New("--prompt-id is required") + return nil, errors.New("--prompt is required") } if len(cfg.inputRaw) == 0 { return nil, errors.New("at least one --input is required") } + cfg.promptDir = filepath.Clean(cfg.promptDir) cfg.profileDir = filepath.Clean(cfg.profileDir) cfg.schemaDir = filepath.Clean(cfg.schemaDir) if cfg.outputPath != "" { @@ -254,6 +270,8 @@ func parseRunArgs(args []string) (*runConfig, error) { cfg.modelSet = flagWasSet(fs, "model") cfg.temperatureSet = flagWasSet(fs, "temperature") cfg.maxTokensSet = flagWasSet(fs, "max-tokens") + cfg.topPSet = flagWasSet(fs, "top-p") + cfg.timeoutSet = flagWasSet(fs, "timeout") return cfg, nil } @@ -264,7 +282,8 @@ func parseServeArgs(args []string) (*serveConfig, error) { fs.SetOutput(io.Discard) fs.StringVar(&cfg.addr, "addr", defaults.HTTPAddrDefault, "HTTP listen address") - fs.StringVar(&cfg.profileDir, "profile-dir", "", "directory containing prompt definition YAML files") + fs.StringVar(&cfg.promptDir, "prompt-dir", "", "directory containing prompt definition YAML files") + fs.StringVar(&cfg.profileDir, "profile-dir", "", "directory containing execution profile YAML files") fs.StringVar(&cfg.schemaDir, "schema-dir", defaults.SchemaDirDefault, "base directory for validation schemas") fs.StringVar(&cfg.llmBaseURL, "llm-base-url", "", "OpenAI-compatible base URL including /v1") fs.StringVar(&cfg.model, "model", "", "optional default model") @@ -277,13 +296,14 @@ func parseServeArgs(args []string) (*serveConfig, error) { return nil, fmt.Errorf("unexpected positional args: %v", fs.Args()) } + if strings.TrimSpace(cfg.promptDir) == "" { + return nil, errors.New("--prompt-dir is required") + } if strings.TrimSpace(cfg.profileDir) == "" { return nil, errors.New("--profile-dir is required") } - if strings.TrimSpace(cfg.llmBaseURL) == "" { - return nil, errors.New("--llm-base-url is required") - } + cfg.promptDir = filepath.Clean(cfg.promptDir) cfg.profileDir = filepath.Clean(cfg.profileDir) cfg.schemaDir = filepath.Clean(cfg.schemaDir) return cfg, nil @@ -376,6 +396,6 @@ func printSummary(stderr io.Writer, res *domain.RunResult) { func printUsage(w io.Writer) { fmt.Fprintln(w, "usage: scriptorium ...") - fmt.Fprintln(w, " run: scriptorium run --profile-dir DIR --prompt-id ID --input name=path [--input ...] [--llm-base-url URL] [--model NAME] [--api-key-env ENV] [--var k=v] [--out path] [--timeout 10m]") - fmt.Fprintln(w, " serve: scriptorium serve --addr :8080 --profile-dir DIR --llm-base-url URL [--schema-dir DIR] [--model NAME] [--timeout 10m]") + fmt.Fprintln(w, " run: scriptorium run --prompt-dir DIR --profile-dir DIR --prompt ID --input name=path [--input ...] [--profile ID] [--llm-base-url URL] [--model NAME] [--api-key-env ENV] [--temperature N] [--max-tokens N] [--top-p N] [--var k=v] [--out path] [--timeout 10m]") + fmt.Fprintln(w, " serve: scriptorium serve --addr :8080 --prompt-dir DIR --profile-dir DIR [--llm-base-url URL] [--schema-dir DIR] [--model NAME] [--timeout 10m]") } diff --git a/internal/adapter/cli/run_test.go b/internal/adapter/cli/run_test.go index 6e8e528..8c22281 100644 --- a/internal/adapter/cli/run_test.go +++ b/internal/adapter/cli/run_test.go @@ -3,10 +3,18 @@ package cli import ( "bytes" "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" "strings" + "sync/atomic" "testing" "time" + "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" "gitea.maximumdirect.net/eric/scriptorium/internal/domain" ) @@ -51,26 +59,61 @@ func TestParseMappingsMalformed(t *testing.T) { } func TestParseRunArgsRequiredFlags(t *testing.T) { - _, err := parseRunArgs([]string{"--prompt-id", "p", "--input", "a=b", "--llm-base-url", "http://x/v1", "--model", "m"}) + _, err := parseRunArgs([]string{"--profile-dir", "./profiles", "--prompt", "p", "--input", "a=b"}) + if err == nil { + t.Fatal("expected missing --prompt-dir error") + } + + _, err = parseRunArgs([]string{"--prompt-dir", "./prompts", "--prompt", "p", "--input", "a=b"}) if err == nil { t.Fatal("expected missing --profile-dir error") } - _, err = parseRunArgs([]string{"--profile-dir", "./profiles", "--input", "a=b", "--llm-base-url", "http://x/v1", "--model", "m"}) + _, err = parseRunArgs([]string{"--prompt-dir", "./prompts", "--profile-dir", "./profiles", "--input", "a=b"}) if err == nil { - t.Fatal("expected missing --prompt-id error") + t.Fatal("expected missing --prompt error") } - _, err = parseRunArgs([]string{"--profile-dir", "./profiles", "--prompt-id", "p", "--llm-base-url", "http://x/v1", "--model", "m"}) + _, err = parseRunArgs([]string{"--prompt-dir", "./prompts", "--profile-dir", "./profiles", "--prompt", "p"}) if err == nil { t.Fatal("expected missing --input error") } } +func TestParseRunArgsFlagMapping(t *testing.T) { + cfg, err := parseRunArgs([]string{ + "--prompt-dir", "./prompts", + "--profile-dir", "./profiles", + "--prompt", "prompt.a", + "--profile", "profile.a", + "--input", "a=b", + "--llm-base-url", "http://x/v1", + "--model", "m", + "--temperature", "0.7", + "--max-tokens", "111", + "--top-p", "0.8", + "--timeout", "30s", + "--api-key-env", "SCRIPTORIUM_API_KEY", + }) + if err != nil { + t.Fatalf("expected valid args, got %v", err) + } + if cfg.promptDir != filepath.Clean("./prompts") || cfg.profileDir != filepath.Clean("./profiles") { + t.Fatalf("unexpected dirs: prompt=%q profile=%q", cfg.promptDir, cfg.profileDir) + } + if cfg.promptID != "prompt.a" || cfg.profileID != "profile.a" { + t.Fatalf("unexpected prompt/profile ids: %q %q", cfg.promptID, cfg.profileID) + } + if !cfg.llmBaseURLSet || !cfg.modelSet || !cfg.temperatureSet || !cfg.maxTokensSet || !cfg.topPSet || !cfg.timeoutSet || !cfg.apiKeyEnvSet { + t.Fatalf("expected override flags set, got %+v", cfg) + } +} + func TestParseRunArgsAllowsOmittedModelAndBaseURL(t *testing.T) { cfg, err := parseRunArgs([]string{ + "--prompt-dir", "./prompts", "--profile-dir", "./profiles", - "--prompt-id", "p", + "--prompt", "p", "--input", "a=b", }) if err != nil { @@ -81,50 +124,61 @@ func TestParseRunArgsAllowsOmittedModelAndBaseURL(t *testing.T) { } } +func TestParseRunArgsRejectsRawLLMAPIKeyFlag(t *testing.T) { + _, err := parseRunArgs([]string{ + "--prompt-dir", "./prompts", + "--profile-dir", "./profiles", + "--prompt", "p", + "--input", "a=b", + "--llm-api-key", "secret", + }) + if err == nil { + t.Fatal("expected unknown flag error for --llm-api-key") + } +} + func TestParseServeArgsRequiredFlags(t *testing.T) { - _, err := parseServeArgs([]string{"--llm-base-url", "http://x/v1"}) + _, err := parseServeArgs([]string{"--profile-dir", "./profiles"}) + if err == nil { + t.Fatal("expected missing --prompt-dir error") + } + + _, err = parseServeArgs([]string{"--prompt-dir", "./prompts"}) if err == nil { t.Fatal("expected missing --profile-dir error") } - _, err = parseServeArgs([]string{"--profile-dir", "./profiles"}) - if err == nil { - t.Fatal("expected missing --llm-base-url error") - } - - cfg, err := parseServeArgs([]string{"--profile-dir", "./profiles", "--llm-base-url", "http://x/v1"}) + cfg, err := parseServeArgs([]string{"--prompt-dir", "./prompts", "--profile-dir", "./profiles"}) if err != nil { t.Fatalf("expected valid serve args, got %v", err) } - if cfg.addr != ":8080" { - t.Fatalf("expected default addr :8080, got %q", cfg.addr) + if cfg.addr != defaults.HTTPAddrDefault { + t.Fatalf("expected default addr %s, got %q", defaults.HTTPAddrDefault, cfg.addr) } - if cfg.timeout != 10*time.Minute { - t.Fatalf("expected default timeout 10m, got %s", cfg.timeout) + if cfg.timeout != defaults.LLMRequestTimeoutDefault { + t.Fatalf("expected default timeout %s, got %s", defaults.LLMRequestTimeoutDefault, cfg.timeout) } } func TestParseRunArgsTimeout(t *testing.T) { cfg, err := parseRunArgs([]string{ + "--prompt-dir", "./prompts", "--profile-dir", "./profiles", - "--prompt-id", "p", + "--prompt", "p", "--input", "a=b", - "--llm-base-url", "http://x/v1", - "--model", "m", }) if err != nil { t.Fatalf("expected valid run args, got %v", err) } - if cfg.timeout != 10*time.Minute { - t.Fatalf("expected default timeout 10m, got %s", cfg.timeout) + if cfg.timeout != defaults.LLMRequestTimeoutDefault { + t.Fatalf("expected default timeout %s, got %s", defaults.LLMRequestTimeoutDefault, cfg.timeout) } cfg, err = parseRunArgs([]string{ + "--prompt-dir", "./prompts", "--profile-dir", "./profiles", - "--prompt-id", "p", + "--prompt", "p", "--input", "a=b", - "--llm-base-url", "http://x/v1", - "--model", "m", "--timeout", "2m30s", }) if err != nil { @@ -155,8 +209,9 @@ func TestRunCommandVarsOptional(t *testing.T) { var stderr bytes.Buffer code := runCommand([]string{ + "--prompt-dir", "./profiles", "--profile-dir", "./profiles", - "--prompt-id", "p", + "--prompt", "p", "--input", "transcript=./t.md", "--llm-base-url", "://bad-url", "--model", "m", @@ -176,6 +231,158 @@ func TestRunCommandVarsOptional(t *testing.T) { } } +func TestRunCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) { + tmp := t.TempDir() + promptDir := filepath.Join(tmp, "prompts") + profileDir := filepath.Join(tmp, "profiles") + if err := os.MkdirAll(promptDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(profileDir, 0o755); err != nil { + t.Fatal(err) + } + inputPath := filepath.Join(tmp, "transcript.md") + if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + + ts := newTestLLMServer("default-output", nil) + defer ts.Close() + + writePromptFile(t, promptDir, "prompt.default", "local-default") + writeProfileFile(t, profileDir, "local-default", ts.URL+"/v1", "profile-model") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runCommand([]string{ + "--prompt-dir", promptDir, + "--profile-dir", profileDir, + "--prompt", "prompt.default", + "--input", "transcript=" + inputPath, + }, &stdout, &stderr) + + if code != ExitOK { + t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String()) + } + if stdout.String() != "default-output" { + t.Fatalf("unexpected stdout output: %q", stdout.String()) + } + if !strings.Contains(stderr.String(), "selected_profile=local-default") { + t.Fatalf("expected selected profile in summary, got %q", stderr.String()) + } +} + +func TestRunCommandExplicitProfileOverridesPromptDefault(t *testing.T) { + tmp := t.TempDir() + promptDir := filepath.Join(tmp, "prompts") + profileDir := filepath.Join(tmp, "profiles") + if err := os.MkdirAll(promptDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(profileDir, 0o755); err != nil { + t.Fatal(err) + } + inputPath := filepath.Join(tmp, "transcript.md") + if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + + defaultServer := newTestLLMServer("from-default", nil) + defer defaultServer.Close() + overrideServer := newTestLLMServer("from-override", nil) + defer overrideServer.Close() + + writePromptFile(t, promptDir, "prompt.default", "local-default") + writeProfileFile(t, profileDir, "local-default", defaultServer.URL+"/v1", "default-model") + writeProfileFile(t, profileDir, "quality", overrideServer.URL+"/v1", "quality-model") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runCommand([]string{ + "--prompt-dir", promptDir, + "--profile-dir", profileDir, + "--prompt", "prompt.default", + "--profile", "quality", + "--input", "transcript=" + inputPath, + }, &stdout, &stderr) + if code != ExitOK { + t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String()) + } + if stdout.String() != "from-override" { + t.Fatalf("expected explicit profile output, got %q", stdout.String()) + } + if !strings.Contains(stderr.String(), "selected_profile=quality") { + t.Fatalf("expected selected profile quality, got %q", stderr.String()) + } +} + +func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) { + tmp := t.TempDir() + promptDir := filepath.Join(tmp, "prompts") + profileDir := filepath.Join(tmp, "profiles") + if err := os.MkdirAll(promptDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(profileDir, 0o755); err != nil { + t.Fatal(err) + } + inputPath := filepath.Join(tmp, "transcript.md") + if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + + var baseHits int32 + baseServer := newTestLLMServer("base", &baseHits) + defer baseServer.Close() + + var overrideHits int32 + var observedBody string + overrideServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&overrideHits, 1) + body, _ := io.ReadAll(r.Body) + observedBody = string(body) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"choices":[{"message":{"role":"assistant","content":"override"}}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2}}`)) + })) + defer overrideServer.Close() + + writePromptFile(t, promptDir, "prompt.default", "local-default") + writeProfileFile(t, profileDir, "local-default", baseServer.URL+"/v1", "profile-model") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runCommand([]string{ + "--prompt-dir", promptDir, + "--profile-dir", profileDir, + "--prompt", "prompt.default", + "--input", "transcript=" + inputPath, + "--llm-base-url", overrideServer.URL + "/v1", + "--model", "override-model", + "--temperature", "0.7", + "--max-tokens", "55", + "--top-p", "0.2", + "--timeout", "20s", + }, &stdout, &stderr) + if code != ExitOK { + t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String()) + } + if atomic.LoadInt32(&baseHits) != 0 { + t.Fatalf("expected base profile endpoint not to be hit, got %d", baseHits) + } + if atomic.LoadInt32(&overrideHits) != 1 { + t.Fatalf("expected override endpoint to be hit once, got %d", overrideHits) + } + if stdout.String() != "override" { + t.Fatalf("unexpected stdout output: %q", stdout.String()) + } + if !strings.Contains(observedBody, `"model":"override-model"`) { + t.Fatalf("expected override model in request body, got %s", observedBody) + } + if !strings.Contains(observedBody, `"temperature":0.7`) || !strings.Contains(observedBody, `"max_tokens":55`) || !strings.Contains(observedBody, `"top_p":0.2`) { + t.Fatalf("expected override generation params in request body, got %s", observedBody) + } +} + func TestWriteOutputAndSummaryUseSeparateWriters(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer @@ -200,3 +407,42 @@ func TestWriteOutputAndSummaryUseSeparateWriters(t *testing.T) { t.Fatalf("expected summary on stderr, got %q", stderr.String()) } } + +func writePromptFile(t *testing.T, dir, id, defaultProfile string) { + t.Helper() + data := fmt.Sprintf(`id: %s +version: "1.0.0" +default_profile: %s +inputs: + - name: transcript + required: true +messages: + - role: user + content: "Summarize: {{input \"transcript\"}}" +output: + format: text + validation_mode: none + repair_attempts: 0 +`, id, defaultProfile) + if err := os.WriteFile(filepath.Join(dir, id+".yaml"), []byte(data), 0o644); err != nil { + t.Fatalf("failed to write prompt fixture: %v", err) + } +} + +func writeProfileFile(t *testing.T, dir, id, endpoint, model string) { + t.Helper() + data := fmt.Sprintf("id: %s\nendpoint: %s\nmodel: %s\n", id, endpoint, model) + if err := os.WriteFile(filepath.Join(dir, id+".yaml"), []byte(data), 0o644); err != nil { + t.Fatalf("failed to write profile fixture: %v", err) + } +} + +func newTestLLMServer(content string, hitCounter *int32) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if hitCounter != nil { + atomic.AddInt32(hitCounter, 1) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(fmt.Sprintf(`{"choices":[{"message":{"role":"assistant","content":%q}}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2}}`, content))) + })) +}