diff --git a/README.md b/README.md index 421900b..e5e43f5 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,9 @@ # scriptorium -Scriptorium is a a prompt-profile execution engine written in Go. \ No newline at end of file +Scriptorium is a prompt-profile execution engine written in Go. + +Current implementation scope: +- domain model and core interfaces +- YAML-backed prompt profile loading +- inline and file artifact reading +- provider-neutral prompt rendering diff --git a/internal/artifact/reader.go b/internal/artifact/reader.go index 584562f..1337359 100644 --- a/internal/artifact/reader.go +++ b/internal/artifact/reader.go @@ -36,6 +36,12 @@ func NewCompositeReader() Reader { } func (c *CompositeReader) Read(ctx context.Context, ref domain.ArtifactRef) (*domain.Artifact, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + switch ref.Type { case domain.ArtifactRefInline: return c.inlineReader.Read(ctx, ref) @@ -49,22 +55,35 @@ func (c *CompositeReader) Read(ctx context.Context, ref domain.ArtifactRef) (*do type inlineReader struct{} func (r *inlineReader) Read(ctx context.Context, ref domain.ArtifactRef) (*domain.Artifact, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + if ref.Body == "" { return nil, ErrMissingInlineBody } body := []byte(ref.Body) return &domain.Artifact{ - Body: body, - Size: int64(len(body)), - Hash: fmt.Sprintf("%x", sha256.Sum256(body)), - URI: ref.URI, + ContentType: "text/plain", + Body: body, + Size: int64(len(body)), + Hash: fmt.Sprintf("%x", sha256.Sum256(body)), + URI: ref.URI, }, nil } type fileReader struct{} func (r *fileReader) Read(ctx context.Context, ref domain.ArtifactRef) (*domain.Artifact, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + if ref.URI == "" { return nil, ErrMissingFilePath } diff --git a/internal/artifact/reader_test.go b/internal/artifact/reader_test.go index 943b896..058a171 100644 --- a/internal/artifact/reader_test.go +++ b/internal/artifact/reader_test.go @@ -2,6 +2,7 @@ package artifact import ( "context" + "errors" "os" "testing" @@ -24,8 +25,11 @@ func TestCompositeReader_Read(t *testing.T) { if string(art.Body) != "hello world" { t.Errorf("expected 'hello world', got %s", string(art.Body)) } - if art.Hash == "" { - t.Error("expected hash to be computed") + if art.ContentType != "text/plain" { + t.Errorf("expected text/plain content type, got %q", art.ContentType) + } + if art.Hash != "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" { + t.Errorf("unexpected hash: %s", art.Hash) } }) @@ -35,7 +39,7 @@ func TestCompositeReader_Read(t *testing.T) { Body: "", } _, err := reader.Read(ctx, ref) - if err == nil || err != ErrMissingInlineBody { + if !errors.Is(err, ErrMissingInlineBody) { t.Errorf("expected ErrMissingInlineBody, got %v", err) } }) @@ -46,7 +50,7 @@ func TestCompositeReader_Read(t *testing.T) { URI: "s3://bucket/key", } _, err := reader.Read(ctx, ref) - if err == nil { + if !errors.Is(err, ErrUnsupportedRefType) { t.Error("expected error for unsupported type") } }) @@ -83,8 +87,8 @@ func TestFileReader_Read(t *testing.T) { if art.Name == "" { t.Error("expected name to be inferred from filename") } - if art.Hash == "" { - t.Error("expected hash to be computed") + if art.Hash != "60f5237ed4049f0382661ef009d2bc42e48c3ceb3edb6600f7024e7ab3b838f3" { + t.Errorf("unexpected hash: %s", art.Hash) } }) @@ -94,7 +98,7 @@ func TestFileReader_Read(t *testing.T) { URI: "", } _, err := reader.Read(ctx, ref) - if err == nil || err != ErrMissingFilePath { + if !errors.Is(err, ErrMissingFilePath) { t.Errorf("expected ErrMissingFilePath, got %v", err) } }) diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 9710e57..43e47c3 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -43,11 +43,13 @@ const ( // RunRequest represents a request to generate a single artifact. type RunRequest struct { - ProfileID string - Inputs map[string]ArtifactRef - Vars map[string]string - Model *ModelTarget - Metadata map[string]string + ProfileID string + ProfileVersion string + Inputs map[string]ArtifactRef + Vars map[string]string + Model *ModelTarget + Validation *OutputContract + Metadata map[string]string } // RunResult represents the complete result of a prompt execution run. diff --git a/internal/profile/filesystem_repository.go b/internal/profile/filesystem_repository.go index 9632ad5..5bdaef1 100644 --- a/internal/profile/filesystem_repository.go +++ b/internal/profile/filesystem_repository.go @@ -1,6 +1,7 @@ package profile import ( + "bytes" "context" "errors" "fmt" @@ -26,12 +27,22 @@ func NewFilesystemRepository(dir string) Repository { } func (r *filesystemRepository) GetProfile(ctx context.Context, id string, version string) (*domain.PromptProfile, error) { + if strings.TrimSpace(id) == "" { + return nil, fmt.Errorf("%w: profile id is required", ErrInvalidProfile) + } + files, err := os.ReadDir(r.dir) if err != nil { return nil, fmt.Errorf("failed to read profile directory: %w", err) } for _, file := range files { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + if file.IsDir() || (!strings.HasSuffix(file.Name(), ".yaml") && !strings.HasSuffix(file.Name(), ".yml")) { continue } @@ -43,8 +54,10 @@ func (r *filesystemRepository) GetProfile(ctx context.Context, id string, versio } var prof domain.PromptProfile - if err := yaml.Unmarshal(data, &prof); err != nil { - if strings.Contains(file.Name(), id) { + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(&prof); err != nil { + if strings.TrimSuffix(strings.TrimSuffix(file.Name(), ".yaml"), ".yml") == id { return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, file.Name(), err) } continue @@ -75,8 +88,8 @@ func validateProfile(p *domain.PromptProfile) error { return errors.New("at least one prompt template message is required") } for i, t := range p.Templates { - if t.Role == "" { - return fmt.Errorf("template message %d is missing role", i) + if !isValidMessageRole(t.Role) { + return fmt.Errorf("template message %d has invalid role %q", i, t.Role) } if t.Content == "" { return fmt.Errorf("template message %d is missing content", i) @@ -88,6 +101,15 @@ func validateProfile(p *domain.PromptProfile) error { 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) + } for i, input := range p.ExpectedInputs { if strings.TrimSpace(input) == "" { return fmt.Errorf("expected input %d has empty name", i) @@ -111,3 +133,11 @@ func isValidValidationMode(m domain.ValidationMode) bool { } return false } + +func isValidMessageRole(role string) bool { + switch role { + case "system", "user", "assistant", "developer": + return true + } + return false +} diff --git a/internal/profile/repository_test.go b/internal/profile/repository_test.go index a99b48c..1e887aa 100644 --- a/internal/profile/repository_test.go +++ b/internal/profile/repository_test.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" "testing" + + "gitea.maximumdirect.net/eric/scriptorium/internal/domain" ) func TestFilesystemRepository_GetProfile(t *testing.T) { @@ -44,6 +46,24 @@ func TestFilesystemRepository_GetProfile(t *testing.T) { if p == nil || p.ID != "test-profile" { t.Errorf("expected profile test-profile, got %v", p) } + if p.Version != "1.0.0" { + t.Fatalf("expected version 1.0.0, got %q", p.Version) + } + if len(p.ExpectedInputs) != 2 || p.ExpectedInputs[0] != "transcript" || p.ExpectedInputs[1] != "glossary" { + t.Fatalf("unexpected expected_inputs: %#v", p.ExpectedInputs) + } + 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) + } }) t.Run("invalid YAML", func(t *testing.T) { @@ -67,6 +87,13 @@ func TestFilesystemRepository_GetProfile(t *testing.T) { } }) + t.Run("json schema mode missing schema path", func(t *testing.T) { + _, err := repo.GetProfile(ctx, "json-schema-missing-path", "") + if !errors.Is(err, ErrInvalidProfile) { + t.Errorf("expected ErrInvalidProfile for json_schema profile without schema_path, got %v", err) + } + }) + t.Run("profile not found", func(t *testing.T) { _, err := repo.GetProfile(ctx, "unknown", "") if !errors.Is(err, ErrProfileNotFound) { diff --git a/internal/profile/testdata/json_schema_missing_path.yaml b/internal/profile/testdata/json_schema_missing_path.yaml new file mode 100644 index 0000000..be19267 --- /dev/null +++ b/internal/profile/testdata/json_schema_missing_path.yaml @@ -0,0 +1,8 @@ +id: json-schema-missing-path +version: "1.0.0" +templates: + - role: user + content: "Return JSON" +output_format: json +validation: + validation_mode: json_schema diff --git a/internal/profile/testdata/valid.yaml b/internal/profile/testdata/valid.yaml index 9bbe30d..1881ba4 100644 --- a/internal/profile/testdata/valid.yaml +++ b/internal/profile/testdata/valid.yaml @@ -1,14 +1,14 @@ id: test-profile -version: 1.0.0 +version: "1.0.0" description: A valid test profile expected_inputs: - transcript - glossary templates: - role: system - content: You are a helpful assistant. + content: "You are a helpful assistant." - role: user - content: Analyze this: {{.transcript}} + content: 'Analyze this: {{input "transcript"}}' model_defaults: model: gpt-4o temperature: 0.7 diff --git a/internal/prompt/go_renderer.go b/internal/prompt/go_renderer.go index 0047aae..e96f3e0 100644 --- a/internal/prompt/go_renderer.go +++ b/internal/prompt/go_renderer.go @@ -13,6 +13,7 @@ var ( ErrMissingRequiredInput = errors.New("missing required input artifact") ErrUnknownInput = errors.New("referenced unknown input artifact") ErrInvalidTemplate = errors.New("invalid prompt template") + ErrRenderFailure = errors.New("prompt render failure") ErrInvalidMessageRole = errors.New("invalid or empty message role") ) @@ -23,9 +24,14 @@ func NewGoRenderer() Renderer { } func (r *goRenderer) Render(ctx context.Context, profile *domain.PromptProfile, inputs map[string]*domain.Artifact, vars map[string]string) (*domain.RenderedPrompt, error) { + if profile == nil { + return nil, fmt.Errorf("%w: nil profile", ErrRenderFailure) + } + // 1. Verify required inputs for _, req := range profile.ExpectedInputs { - if _, ok := inputs[req]; !ok { + art, ok := inputs[req] + if !ok || art == nil { return nil, fmt.Errorf("%w: %s", ErrMissingRequiredInput, req) } } @@ -44,19 +50,25 @@ func (r *goRenderer) Render(ctx context.Context, profile *domain.PromptProfile, var renderedMessages []domain.RenderedMessage for i, tmplMsg := range profile.Templates { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + if tmplMsg.Role == "" { return nil, fmt.Errorf("%w: message %d", ErrInvalidMessageRole, i) } // Parse and execute template - tmpl, err := template.New(fmt.Sprintf("msg_%d", i)).Funcs(funcs).Parse(tmplMsg.Content) + tmpl, err := template.New(fmt.Sprintf("msg_%d", i)).Funcs(funcs).Option("missingkey=error").Parse(tmplMsg.Content) if err != nil { return nil, fmt.Errorf("%w: message %d: %v", ErrInvalidTemplate, i, err) } var buf bytes.Buffer if err := tmpl.Execute(&buf, vars); err != nil { - return nil, fmt.Errorf("execution failed for message %d: %v", i, err) + return nil, fmt.Errorf("%w: message %d: %w", ErrRenderFailure, i, err) } renderedMessages = append(renderedMessages, domain.RenderedMessage{ diff --git a/internal/prompt/renderer_test.go b/internal/prompt/renderer_test.go index 3712567..e129db0 100644 --- a/internal/prompt/renderer_test.go +++ b/internal/prompt/renderer_test.go @@ -2,6 +2,7 @@ package prompt import ( "context" + "errors" "testing" "gitea.maximumdirect.net/eric/scriptorium/internal/domain" @@ -47,7 +48,7 @@ func TestGoRenderer_Render(t *testing.T) { t.Run("missing required input", func(t *testing.T) { emptyInputs := map[string]*domain.Artifact{} _, err := renderer.Render(ctx, profile, emptyInputs, vars) - if err == nil || (err != ErrMissingRequiredInput && err.Error() != "missing required input artifact: transcript") { + if !errors.Is(err, ErrMissingRequiredInput) { t.Errorf("expected ErrMissingRequiredInput, got %v", err) } }) @@ -59,8 +60,11 @@ func TestGoRenderer_Render(t *testing.T) { }, } _, err := renderer.Render(ctx, profileUnknown, inputs, vars) - if err == nil { - t.Error("expected error for unknown input") + if !errors.Is(err, ErrRenderFailure) { + t.Errorf("expected ErrRenderFailure, got %v", err) + } + if !errors.Is(err, ErrUnknownInput) { + t.Errorf("expected ErrUnknownInput, got %v", err) } }) @@ -71,8 +75,8 @@ func TestGoRenderer_Render(t *testing.T) { }, } _, err := renderer.Render(ctx, profileInvalid, inputs, vars) - if err == nil { - t.Error("expected error for invalid template syntax") + if !errors.Is(err, ErrInvalidTemplate) { + t.Errorf("expected ErrInvalidTemplate, got %v", err) } }) @@ -83,8 +87,20 @@ func TestGoRenderer_Render(t *testing.T) { }, } _, err := renderer.Render(ctx, profileNoRole, inputs, vars) - if err == nil { - t.Error("expected error for empty message role") + if !errors.Is(err, ErrInvalidMessageRole) { + t.Errorf("expected ErrInvalidMessageRole, got %v", err) + } + }) + + t.Run("missing variable in template", func(t *testing.T) { + profileMissingVar := &domain.PromptProfile{ + Templates: []domain.PromptMessageTemplate{ + {Role: "system", Content: "You are {{.missing}}"}, + }, + } + _, err := renderer.Render(ctx, profileMissingVar, inputs, vars) + if !errors.Is(err, ErrRenderFailure) { + t.Errorf("expected ErrRenderFailure for missing variable, got %v", err) } }) }