Refactor: load execution profiles from YAML and split prompt definitions into promptdef repository

internal/profile/filesystem_repository.go

@@ -5,17 +5,19 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
-	"gopkg.in/yaml.v3"
 	"os"
 	"path/filepath"
 	"strings"
+
+	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
+	"gopkg.in/yaml.v3"
 )
 
 var (
-	ErrProfileNotFound = errors.New("prompt definition not found")
-	ErrInvalidYAML     = errors.New("invalid YAML format")
-	ErrInvalidProfile  = errors.New("invalid prompt definition configuration")
+	ErrProfileNotFound     = errors.New("execution profile not found")
+	ErrInvalidYAML         = errors.New("invalid YAML format")
+	ErrInvalidProfile      = errors.New("invalid execution profile configuration")
+	ErrRawAPIKeyNotAllowed = errors.New("raw api_key is not allowed; use api_key_env")
 )
 
 type filesystemRepository struct {
@@ -26,9 +28,9 @@ func NewFilesystemRepository(dir string) Repository {
 	return &filesystemRepository{dir: dir}
 }
 
-func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id string, version string) (*domain.PromptDefinition, error) {
+func (r *filesystemRepository) GetProfile(ctx context.Context, id string) (*domain.ExecutionProfile, error) {
 	if strings.TrimSpace(id) == "" {
-		return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidProfile)
+		return nil, fmt.Errorf("%w: profile id is required", ErrInvalidProfile)
 	}
 
 	files, err := os.ReadDir(r.dir)
@@ -53,97 +55,60 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
 			return nil, fmt.Errorf("failed to read profile file %s: %w", file.Name(), err)
 		}
 
-		var prof domain.PromptDefinition
+		var prof domain.ExecutionProfile
 		decoder := yaml.NewDecoder(bytes.NewReader(data))
 		decoder.KnownFields(true)
 		if err := decoder.Decode(&prof); err != nil {
+			if strings.Contains(err.Error(), "field api_key not found") {
+				if strings.TrimSuffix(strings.TrimSuffix(file.Name(), ".yaml"), ".yml") == id {
+					return nil, fmt.Errorf("%w: %s", ErrRawAPIKeyNotAllowed, file.Name())
+				}
+				continue
+			}
 			if strings.TrimSuffix(strings.TrimSuffix(file.Name(), ".yaml"), ".yml") == id {
 				return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, file.Name(), err)
 			}
 			continue
 		}
 
-		if prof.ID == id {
-			if version == "" || prof.Version == version {
-				if err := validateProfile(&prof); err != nil {
-					return nil, fmt.Errorf("%w: %s: %v", ErrInvalidProfile, file.Name(), err)
-				}
-				return &prof, nil
-			}
+		if prof.ID != id {
+			continue
 		}
-
+		if err := validateProfile(&prof); err != nil {
+			if errors.Is(err, ErrRawAPIKeyNotAllowed) {
+				return nil, fmt.Errorf("%w: %s", err, file.Name())
+			}
+			return nil, fmt.Errorf("%w: %s: %v", ErrInvalidProfile, file.Name(), err)
+		}
+		return &prof, nil
 	}
 
 	return nil, ErrProfileNotFound
 }
 
-func validateProfile(p *domain.PromptDefinition) error {
-	if p.ID == "" {
-		return errors.New("prompt id is required")
+func validateProfile(p *domain.ExecutionProfile) error {
+	if strings.TrimSpace(p.ID) == "" {
+		return errors.New("id is required")
 	}
-	if p.Version == "" {
-		return errors.New("prompt version is required")
+	if strings.TrimSpace(p.Endpoint) == "" {
+		return errors.New("endpoint is required")
 	}
-	if len(p.Templates) == 0 {
-		return errors.New("at least one prompt template message is required")
+	if strings.TrimSpace(p.Model) == "" {
+		return errors.New("model is required")
 	}
-	if len(p.Inputs) == 0 {
-		return errors.New("at least one prompt input is required")
+
+	if p.Temperature < 0 || p.Temperature > 2 {
+		return errors.New("temperature must be between 0 and 2")
 	}
-	for i, input := range p.Inputs {
-		if strings.TrimSpace(input.Name) == "" {
-			return fmt.Errorf("input %d has empty name", i)
-		}
+	if p.MaxTokens < 0 {
+		return errors.New("max_tokens must be greater than or equal to 0")
 	}
-	for i, t := range p.Templates {
-		if !isValidMessageRole(t.Role) {
-			return fmt.Errorf("template message %d has invalid role %q", i, t.Role)
-		}
-		if strings.TrimSpace(t.Content) == "" && strings.TrimSpace(t.ContentFile) == "" {
-			return fmt.Errorf("template message %d must provide content or content_file", i)
-		}
-		if strings.TrimSpace(t.Content) != "" && strings.TrimSpace(t.ContentFile) != "" {
-			return fmt.Errorf("template message %d cannot set both content and content_file", i)
-		}
+	if p.TopP < 0 || p.TopP > 1 {
+		return errors.New("top_p must be between 0 and 1")
 	}
-	if !isValidOutputFormat(p.OutputFormat) {
-		return fmt.Errorf("invalid output format: %s", p.OutputFormat)
-	}
-	if !isValidValidationMode(p.Validation.ValidationMode) {
-		return fmt.Errorf("invalid validation mode: %s", p.Validation.ValidationMode)
-	}
-	if p.Validation.RepairAttempts < 0 {
-		return errors.New("validation.repair_attempts must be greater than or equal to 0")
-	}
-	if p.Validation.ValidationMode == domain.ValidationJSONSchema && strings.TrimSpace(p.Validation.SchemaPath) == "" {
-		return errors.New("validation.schema_path is required when validation_mode is json_schema")
-	}
-	if p.Validation.Format != "" && p.Validation.Format != p.OutputFormat {
-		return fmt.Errorf("validation format %q does not match output format %q", p.Validation.Format, p.OutputFormat)
+	if p.TimeoutSeconds < 0 {
+		return errors.New("timeout_seconds must be greater than or equal to 0")
 	}
+
 	return nil
 }
-
-func isValidOutputFormat(f domain.OutputFormat) bool {
-	switch f {
-	case domain.FormatText, domain.FormatMarkdown, domain.FormatJSON:
-		return true
-	}
-	return false
-}
-
-func isValidValidationMode(m domain.ValidationMode) bool {
-	switch m {
-	case domain.ValidationNone, domain.ValidationBasic, domain.ValidationJSON, domain.ValidationJSONSchema:
-		return true
-	}
-	return false
-}
-
-func isValidMessageRole(role string) bool {
-	switch role {
-	case "system", "user", "assistant", "developer":
-		return true
-	}
-	return false
-}

internal/profile/repository.go

@@ -2,12 +2,11 @@ package profile
 
 import (
 	"context"
+
 	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
 )
 
-// Repository is a transitional prompt-definition repository.
-// It currently lives in internal/profile until package responsibilities
-// are split in a follow-up refactor.
+// Repository loads execution profiles.
 type Repository interface {
-	GetPromptDefinition(ctx context.Context, id string, version string) (*domain.PromptDefinition, error)
+	GetProfile(ctx context.Context, id string) (*domain.ExecutionProfile, error)
 }

internal/profile/repository_test.go

@@ -6,25 +6,21 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-
-	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
 )
 
-func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
-	tmpDir, err := os.MkdirTemp("", "profile_test")
+func TestFilesystemRepository_GetProfile(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "execution_profile_test")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(tmpDir)
 
-	testDataDir := "testdata"
-	files, err := os.ReadDir(testDataDir)
+	files, err := os.ReadDir("testdata")
 	if err != nil {
 		t.Fatalf("failed to read testdata: %v", err)
 	}
-
 	for _, f := range files {
-		src := filepath.Join(testDataDir, f.Name())
+		src := filepath.Join("testdata", f.Name())
 		dst := filepath.Join(tmpDir, f.Name())
 		data, err := os.ReadFile(src)
 		if err != nil {
@@ -38,69 +34,78 @@ func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
 	repo := NewFilesystemRepository(tmpDir)
 	ctx := context.Background()
 
-	t.Run("valid prompt definition", func(t *testing.T) {
-		p, err := repo.GetPromptDefinition(ctx, "test-profile", "")
+	t.Run("valid local profile", func(t *testing.T) {
+		p, err := repo.GetProfile(ctx, "local-default")
 		if err != nil {
 			t.Fatalf("expected no error, got %v", err)
 		}
-		if p == nil || p.ID != "test-profile" {
-			t.Errorf("expected prompt definition test-profile, got %v", p)
+		if p.ID != "local-default" {
+			t.Fatalf("unexpected id: %q", p.ID)
 		}
-		if p.Version != "1.0.0" {
-			t.Fatalf("expected version 1.0.0, got %q", p.Version)
-		}
-		if len(p.Inputs) != 2 || p.Inputs[0].Name != "transcript" || p.Inputs[1].Name != "glossary" {
-			t.Fatalf("unexpected inputs: %#v", p.Inputs)
-		}
-		if len(p.Templates) != 2 {
-			t.Fatalf("expected 2 templates, got %d", len(p.Templates))
-		}
-		if p.Templates[0].Role != "system" || p.Templates[1].Role != "user" {
-			t.Fatalf("unexpected template roles: %#v", p.Templates)
-		}
-		if p.OutputFormat != domain.FormatMarkdown {
-			t.Fatalf("expected output format markdown, got %q", p.OutputFormat)
-		}
-		if p.Validation.ValidationMode != domain.ValidationBasic {
-			t.Fatalf("expected validation mode basic, got %q", p.Validation.ValidationMode)
-		}
-		if p.DefaultProfile != "test-exec" {
-			t.Fatalf("expected default profile test-exec, got %q", p.DefaultProfile)
+		if p.Endpoint == "" || p.Model == "" {
+			t.Fatalf("expected endpoint/model to be set: %+v", p)
 		}
 	})
 
-	t.Run("invalid YAML", func(t *testing.T) {
-		_, err := repo.GetPromptDefinition(ctx, "invalid_yaml", "")
+	t.Run("valid profile with api_key_env", func(t *testing.T) {
+		p, err := repo.GetProfile(ctx, "local-secure")
+		if err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+		if p.APIKeyEnv != "SCRIPTORIUM_API_KEY" {
+			t.Fatalf("unexpected api_key_env: %q", p.APIKeyEnv)
+		}
+		if p.ReasoningEffort != "medium" {
+			t.Fatalf("unexpected reasoning_effort: %q", p.ReasoningEffort)
+		}
+	})
+
+	t.Run("invalid yaml", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "invalid_yaml")
 		if !errors.Is(err, ErrInvalidYAML) {
-			t.Errorf("expected ErrInvalidYAML, got %v", err)
+			t.Fatalf("expected ErrInvalidYAML, got %v", err)
 		}
 	})
 
-	t.Run("missing ID", func(t *testing.T) {
-		_, err := repo.GetPromptDefinition(ctx, "missing-id", "")
+	t.Run("missing id", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "missing_id")
 		if !errors.Is(err, ErrProfileNotFound) {
-			t.Errorf("expected ErrProfileNotFound for profile with missing ID, got %v", err)
+			t.Fatalf("expected ErrProfileNotFound, got %v", err)
 		}
 	})
 
-	t.Run("no templates", func(t *testing.T) {
-		_, err := repo.GetPromptDefinition(ctx, "no-templates", "")
+	t.Run("missing endpoint", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "missing-endpoint")
 		if !errors.Is(err, ErrInvalidProfile) {
-			t.Errorf("expected ErrInvalidProfile for profile with no templates, got %v", err)
+			t.Fatalf("expected ErrInvalidProfile, got %v", err)
 		}
 	})
 
-	t.Run("json schema mode missing schema path", func(t *testing.T) {
-		_, err := repo.GetPromptDefinition(ctx, "json-schema-missing-path", "")
+	t.Run("missing model", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "missing-model")
 		if !errors.Is(err, ErrInvalidProfile) {
-			t.Errorf("expected ErrInvalidProfile for json_schema profile without schema_path, got %v", err)
+			t.Fatalf("expected ErrInvalidProfile, got %v", err)
 		}
 	})
 
-	t.Run("prompt definition not found", func(t *testing.T) {
-		_, err := repo.GetPromptDefinition(ctx, "unknown", "")
+	t.Run("unknown field", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "unknown_field")
+		if !errors.Is(err, ErrInvalidYAML) {
+			t.Fatalf("expected ErrInvalidYAML for strict decode unknown field, got %v", err)
+		}
+	})
+
+	t.Run("raw api_key rejected", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "raw_api_key")
+		if !errors.Is(err, ErrRawAPIKeyNotAllowed) {
+			t.Fatalf("expected ErrRawAPIKeyNotAllowed, got %v", err)
+		}
+	})
+
+	t.Run("profile not found", func(t *testing.T) {
+		_, err := repo.GetProfile(ctx, "does-not-exist")
 		if !errors.Is(err, ErrProfileNotFound) {
-			t.Errorf("expected ErrProfileNotFound, got %v", err)
+			t.Fatalf("expected ErrProfileNotFound, got %v", err)
 		}
 	})
 }

internal/profile/testdata/invalid_yaml.yaml

@@ -1,5 +1,3 @@
-id: invalid-yaml
-version: 1.0.0
-templates:
-  - role: system
-    content: [unclosed bracket
+id: invalid_yaml
+endpoint: http://localhost:8000/v1
+model: [broken

internal/profile/testdata/missing_endpoint.yaml

@@ -0,0 +1,2 @@
+id: missing-endpoint
+model: gpt-4o-mini

internal/profile/testdata/missing_id.yaml

@@ -1,11 +1,2 @@
-version: 1.0.0
-description: Missing ID
-inputs:
-  - name: transcript
-    required: true
-templates:
-  - role: system
-    content: Hello
-output_format: text
-validation:
-  validation_mode: none
+endpoint: http://localhost:8000/v1
+model: gpt-4o-mini

internal/profile/testdata/missing_model.yaml

@@ -0,0 +1,2 @@
+id: missing-model
+endpoint: http://localhost:8000/v1

internal/profile/testdata/raw_api_key.yaml

@@ -0,0 +1,4 @@
+id: raw-api-key
+endpoint: http://localhost:8000/v1
+model: gpt-4o-mini
+api_key: super-secret-should-not-be-here

internal/profile/testdata/unknown_field.yaml

@@ -0,0 +1,4 @@
+id: unknown-field
+endpoint: http://localhost:8000/v1
+model: gpt-4o-mini
+foo: bar

internal/profile/testdata/valid_local_profile.yaml

@@ -0,0 +1,7 @@
+id: local-default
+endpoint: http://localhost:8000/v1
+model: gpt-4o-mini
+temperature: 0.2
+max_tokens: 700
+top_p: 1.0
+timeout_seconds: 120

internal/profile/testdata/valid_with_api_key_env.yaml

@@ -0,0 +1,7 @@
+id: local-secure
+endpoint: http://localhost:8000/v1
+model: gpt-4o-mini
+api_key_env: SCRIPTORIUM_API_KEY
+reasoning_effort: medium
+extra_params:
+  provider: local