Split prompt/profile dirs, adopt --prompt/--profile flags, and add override/selection integration tests

2026-05-05 10:49:33 -05:00
parent 8818796b8b
commit e66763f2a7
2 changed files with 310 additions and 44 deletions
--- 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)))
+	}))
+}