diff --git a/artifact_reader.go b/artifact_reader.go new file mode 100644 index 0000000..74c9e9f --- /dev/null +++ b/artifact_reader.go @@ -0,0 +1,39 @@ +package scriptorium + +import ( + "context" + "errors" + + artifactadapter "gitea.maximumdirect.net/eric/scriptorium/internal/artifact" + "gitea.maximumdirect.net/eric/scriptorium/internal/domain" +) + +var errNilArtifactReaderResponse = errors.New("artifact reader returned nil artifact without error") + +type publicArtifactReaderAdapter struct { + reader ArtifactReader +} + +var _ artifactadapter.Reader = publicArtifactReaderAdapter{} + +func (a publicArtifactReaderAdapter) Read(ctx context.Context, ref domain.ArtifactRef) (*domain.Artifact, error) { + artifact, err := a.reader.Read(ctx, ArtifactRef{ + Type: ArtifactRefType(ref.Type), + URI: ref.URI, + Body: ref.Body, + }) + if err != nil { + return nil, err + } + if artifact == nil { + return nil, errNilArtifactReaderResponse + } + return &domain.Artifact{ + Name: artifact.Name, + ContentType: artifact.ContentType, + Body: copyBytes(artifact.Body), + URI: artifact.URI, + Size: artifact.Size, + Hash: artifact.Hash, + }, nil +} diff --git a/artifact_reader_internal_test.go b/artifact_reader_internal_test.go new file mode 100644 index 0000000..1709e87 --- /dev/null +++ b/artifact_reader_internal_test.go @@ -0,0 +1,36 @@ +package scriptorium + +import ( + "context" + "testing" + + "gitea.maximumdirect.net/eric/scriptorium/internal/domain" +) + +func TestPublicArtifactReaderAdapterCopiesBody(t *testing.T) { + reader := internalArtifactReaderFake{ + artifact: &Artifact{Body: []byte("original")}, + } + adapter := publicArtifactReaderAdapter{reader: &reader} + + artifact, err := adapter.Read(context.Background(), domain.ArtifactRef{ + Type: domain.ArtifactRefInline, + URI: "memory://input", + Body: "input", + }) + if err != nil { + t.Fatalf("read artifact: %v", err) + } + artifact.Body[0] = 'X' + if got := string(reader.artifact.Body); got != "original" { + t.Fatalf("reader artifact body was mutated: %q", got) + } +} + +type internalArtifactReaderFake struct { + artifact *Artifact +} + +func (r *internalArtifactReaderFake) Read(context.Context, ArtifactRef) (*Artifact, error) { + return r.artifact, nil +} diff --git a/docs/consumers/pkg-scriptorium.md b/docs/consumers/pkg-scriptorium.md index f9e64ea..eb6da72 100644 --- a/docs/consumers/pkg-scriptorium.md +++ b/docs/consumers/pkg-scriptorium.md @@ -24,7 +24,8 @@ fields: | `HTTPClient` | Optional HTTP client for that built-in client. It is cloned; a positive `Timeout` on it is the transport cap and takes precedence over `Config.Timeout`. A non-positive client timeout is treated as unset. | Nil options are ignored. Invalid construction, including -`WithLLMClient(nil)`, returns an error matching `ErrInvalidConfig`. +`WithLLMClient(nil)` and `WithArtifactReader(nil)`, returns an error matching +`ErrInvalidConfig`. Profile and request `timeout_seconds` values select a per-generation-call deadline independently of the transport cap. An explicit request override of @@ -38,7 +39,8 @@ Source options replace their matching directory source: - profiles: `WithProfileFS(fsys, root)`, `WithProfileFile(path)`, and `WithProfiles(profiles...)`; - schemas: `WithSchemaFS(fsys, root)`, `WithSchemaFile(path)`; and -- LLM client: `WithLLMClient(client)`. +- LLM client: `WithLLMClient(client)`; and +- artifact reader: `WithArtifactReader(reader)`. `fs.FS` prompt-content and schema paths stay inside their configured roots. Single-file prompt and profile sources are selected by their YAML `id`, not @@ -109,6 +111,14 @@ The exported constants define these serialized values: counts. `RenderedPrompt`, `RenderedMessage`, `CacheControl`, and `StructuredOutputSpec` are the public shapes used by injected LLM clients. +`ArtifactReader` implements +`Read(context.Context, ArtifactRef) (*Artifact, error)`. Supplying it through +`WithArtifactReader` replaces the engine's default inline and file reader for +all inputs. Reader errors remain available through `errors.Is` alongside +`ErrArtifactLoad`; a nil artifact with no error is treated as an artifact-load +failure. Readers must supply artifact metadata and should not retain or mutate +the caller's values. + ## Requests, Inputs, And Overrides `RunRequest` fields are `PromptID`, `PromptVersion`, `ProfileID`, @@ -172,8 +182,10 @@ Public methods preserve these sentinel checks through `errors.Is`: - `ErrInvalidRequest` - `ErrPromptNotFound` - `ErrProfileNotFound` +- `ErrProfileRequired` - `ErrPromptLoad` - `ErrProfileLoad` +- `ErrAPIKeyEnvMissing` - `ErrArtifactLoad` - `ErrPromptRender` - `ErrLLMGenerate` diff --git a/engine.go b/engine.go index a0f53bb..4fcc5c7 100644 --- a/engine.go +++ b/engine.go @@ -26,15 +26,17 @@ import ( var ErrInvalidConfig = errors.New("invalid engine configuration") var ( - ErrInvalidRequest = errors.New("invalid run request") - ErrPromptNotFound = errors.New("prompt not found") - ErrProfileNotFound = errors.New("profile not found") - ErrPromptLoad = errors.New("failed to load prompt definition") - ErrProfileLoad = errors.New("failed to load execution profile") - ErrArtifactLoad = errors.New("failed to load artifact") - ErrPromptRender = errors.New("failed to render prompt") - ErrLLMGenerate = errors.New("failed to generate output") - ErrValidation = errors.New("failed to validate output") + ErrInvalidRequest = errors.New("invalid run request") + ErrPromptNotFound = errors.New("prompt not found") + ErrProfileNotFound = errors.New("profile not found") + ErrProfileRequired = errors.New("profile selection is required") + ErrPromptLoad = errors.New("failed to load prompt definition") + ErrProfileLoad = errors.New("failed to load execution profile") + ErrAPIKeyEnvMissing = errors.New("api_key_env points to an unset environment variable") + ErrArtifactLoad = errors.New("failed to load artifact") + ErrPromptRender = errors.New("failed to render prompt") + ErrLLMGenerate = errors.New("failed to generate output") + ErrValidation = errors.New("failed to validate output") ) // Engine prepares and runs Scriptorium prompt requests. @@ -68,6 +70,7 @@ func (f optionFunc) apply(options *engineOptions) error { type engineOptions struct { llmClient llm.Client + artifactReader artifactadapter.Reader promptDefs promptdef.Repository profiles profile.Repository memoryProfiles profile.Repository @@ -76,6 +79,7 @@ type engineOptions struct { profileSource bool memorySource bool validatorSource bool + artifactSource bool } // WithLLMClient injects a custom LLM client for execution. @@ -89,6 +93,18 @@ func WithLLMClient(client LLMClient) Option { }) } +// WithArtifactReader injects a reader for every input artifact reference. +func WithArtifactReader(reader ArtifactReader) Option { + return optionFunc(func(options *engineOptions) error { + if reader == nil { + return ErrInvalidConfig + } + options.artifactReader = publicArtifactReaderAdapter{reader: reader} + options.artifactSource = true + return nil + }) +} + // WithPromptFS loads prompt definitions from fsys under root. // // The source uses the same strict prompt YAML rules as configured prompt @@ -253,11 +269,16 @@ func NewEngine(cfg Config, opts ...Option) (*Engine, error) { } } + artifacts := options.artifactReader + if !options.artifactSource { + artifacts = artifactadapter.NewCompositeReader() + } + return &Engine{ runner: usecase.NewRunner( promptDefs, profiles, - artifactadapter.NewCompositeReader(), + artifacts, prompt.NewGoRenderer(), llmClient, validator, diff --git a/engine_test.go b/engine_test.go index ef47e86..0d95ae1 100644 --- a/engine_test.go +++ b/engine_test.go @@ -835,11 +835,132 @@ func TestMissingCredentialsFailClearlyWhenProfileRequiresAuth(t *testing.T) { if !errors.Is(err, scriptorium.ErrInvalidRequest) { t.Fatalf("expected invalid request for missing credentials, got %v", err) } + if !errors.Is(err, scriptorium.ErrAPIKeyEnvMissing) { + t.Fatalf("expected missing credential environment error, got %v", err) + } if err == nil || !strings.Contains(err.Error(), missingEnv) { t.Fatalf("expected missing env name in error, got %v", err) } } +func TestWithArtifactReaderRejectsNilReader(t *testing.T) { + _, err := scriptorium.NewEngine(contractConfig(frameworkSchemaDir), scriptorium.WithArtifactReader(nil)) + if !errors.Is(err, scriptorium.ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig, got %v", err) + } +} + +func TestArtifactReaderReceivesPublicReferenceAndPreparesArtifact(t *testing.T) { + reader := &recordingArtifactReader{ + artifact: &scriptorium.Artifact{ + ContentType: "text/plain", + Body: []byte("Reader-supplied transcript."), + URI: "reader://transcript", + Size: int64(len("Reader-supplied transcript.")), + Hash: "reader-transcript-hash", + }, + } + engine := newArtifactReaderEngine(t, reader) + + ref := scriptorium.ArtifactRef{ + Type: scriptorium.ArtifactRefInline, + URI: "reader://transcript", + Body: "request body", + } + prepared, err := engine.Prepare(context.Background(), scriptorium.RunRequest{ + PromptID: "artifact-reader", + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": ref, + }, + }) + if err != nil { + t.Fatalf("prepare with artifact reader: %v", err) + } + if len(reader.refs) != 1 || !reflect.DeepEqual(reader.refs[0], ref) { + t.Fatalf("reader received %#v, want %#v", reader.refs, ref) + } + if prepared.InputHashes["transcript"] != "reader-transcript-hash" { + t.Fatalf("unexpected input hash: %#v", prepared.InputHashes) + } + if len(prepared.Messages) != 1 || !strings.Contains(prepared.Messages[0].Content, "Reader-supplied transcript.") { + t.Fatalf("prepared prompt omitted reader artifact: %#v", prepared.Messages) + } +} + +func TestArtifactReaderFailuresPreserveArtifactLoadErrors(t *testing.T) { + readerErr := errors.New("artifact reader failed") + + tests := []struct { + name string + ctx context.Context + reader *recordingArtifactReader + wantNested error + }{ + { + name: "reader error", + ctx: context.Background(), + reader: &recordingArtifactReader{err: readerErr}, + wantNested: readerErr, + }, + { + name: "nil artifact", + ctx: context.Background(), + reader: &recordingArtifactReader{}, + }, + { + name: "reader cancellation", + ctx: context.Background(), + reader: &recordingArtifactReader{read: func(context.Context, scriptorium.ArtifactRef) (*scriptorium.Artifact, error) { + return nil, context.Canceled + }}, + wantNested: context.Canceled, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + engine := newArtifactReaderEngine(t, tc.reader) + _, err := engine.Prepare(tc.ctx, scriptorium.RunRequest{ + PromptID: "artifact-reader", + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("input"), + }, + }) + if !errors.Is(err, scriptorium.ErrArtifactLoad) { + t.Fatalf("expected ErrArtifactLoad, got %v", err) + } + if tc.wantNested != nil && !errors.Is(err, tc.wantNested) { + t.Fatalf("expected nested %v, got %v", tc.wantNested, err) + } + }) + } +} + +func TestPrepareWithoutProfileMatchesSpecificPublicError(t *testing.T) { + promptDir := t.TempDir() + writePublicPromptFile(t, promptDir, "profile-required", "") + engine, err := scriptorium.NewEngine(scriptorium.Config{ + PromptDir: promptDir, + SchemaDir: frameworkSchemaDir, + }) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + + _, err = engine.Prepare(context.Background(), scriptorium.RunRequest{ + PromptID: "profile-required", + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("input"), + }, + }) + if !errors.Is(err, scriptorium.ErrInvalidRequest) { + t.Fatalf("expected ErrInvalidRequest, got %v", err) + } + if !errors.Is(err, scriptorium.ErrProfileRequired) { + t.Fatalf("expected ErrProfileRequired, got %v", err) + } +} + func TestRunValidationFailureReturnsResult(t *testing.T) { fake := &fakeLLMClient{ response: &scriptorium.GenerateResponse{Content: ""}, @@ -2207,6 +2328,22 @@ func newContractEngineWithOptions(t *testing.T, schemaDir string, opts ...script return engine } +func newArtifactReaderEngine(t *testing.T, reader scriptorium.ArtifactReader) *scriptorium.Engine { + t.Helper() + + promptDir := t.TempDir() + writePublicPromptFile(t, promptDir, "artifact-reader", frameworkFastProfileID) + engine, err := scriptorium.NewEngine(scriptorium.Config{ + PromptDir: promptDir, + ProfileDir: frameworkProfileDir, + SchemaDir: frameworkSchemaDir, + }, scriptorium.WithArtifactReader(reader)) + if err != nil { + t.Fatalf("construct engine with artifact reader: %v", err) + } + return engine +} + func contractConfig(schemaDir string) scriptorium.Config { return scriptorium.Config{ PromptDir: frameworkPromptDir, @@ -2363,6 +2500,24 @@ type fakeLLMClient struct { requests []scriptorium.GenerateRequest } +type recordingArtifactReader struct { + artifact *scriptorium.Artifact + err error + refs []scriptorium.ArtifactRef + read func(context.Context, scriptorium.ArtifactRef) (*scriptorium.Artifact, error) +} + +func (r *recordingArtifactReader) Read(ctx context.Context, ref scriptorium.ArtifactRef) (*scriptorium.Artifact, error) { + r.refs = append(r.refs, ref) + if r.read != nil { + return r.read(ctx, ref) + } + if r.err != nil { + return nil, r.err + } + return r.artifact, nil +} + type roundTripFunc func(*http.Request) (*http.Response, error) func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { diff --git a/errors.go b/errors.go index 492ef51..4e9ca16 100644 --- a/errors.go +++ b/errors.go @@ -29,8 +29,10 @@ func hasPublicError(err error) bool { ErrInvalidRequest, ErrPromptNotFound, ErrProfileNotFound, + ErrProfileRequired, ErrPromptLoad, ErrProfileLoad, + ErrAPIKeyEnvMissing, ErrArtifactLoad, ErrPromptRender, ErrLLMGenerate, @@ -49,6 +51,8 @@ func publicErrorFor(err error) error { return ErrPromptNotFound case errors.Is(err, profile.ErrProfileNotFound): return ErrProfileNotFound + case errors.Is(err, usecase.ErrProfileRequired): + return errors.Join(ErrInvalidRequest, ErrProfileRequired) case errors.Is(err, usecase.ErrPromptLoad): return ErrPromptLoad case errors.Is(err, usecase.ErrProfileLoad): @@ -57,6 +61,8 @@ func publicErrorFor(err error) error { return ErrPromptLoad case isProfileLoadCause(err): return ErrProfileLoad + case errors.Is(err, usecase.ErrAPIKeyEnvMissing): + return errors.Join(ErrInvalidRequest, ErrAPIKeyEnvMissing) case errors.Is(err, usecase.ErrArtifactLoad): return ErrArtifactLoad case errors.Is(err, usecase.ErrPromptRender): diff --git a/types.go b/types.go index 0bcedba..c16cafc 100644 --- a/types.go +++ b/types.go @@ -126,6 +126,14 @@ type Artifact struct { Hash string } +// ArtifactReader resolves a prompt input reference into its content. +// +// Readers are responsible for supplying artifact metadata. The engine assigns +// an input-map name only when the returned artifact name is empty. +type ArtifactReader interface { + Read(context.Context, ArtifactRef) (*Artifact, error) +} + // ExecutionTarget represents effective model runtime settings. type ExecutionTarget struct { Endpoint string `json:"endpoint"`