Documentation cleanup and bugfixes
This commit is contained in:
@@ -61,9 +61,6 @@ type serveConfig struct {
|
||||
promptDir string
|
||||
profileDir string
|
||||
schemaDir string
|
||||
llmBaseURL string
|
||||
model string
|
||||
timeout time.Duration
|
||||
}
|
||||
|
||||
type listFlag []string
|
||||
@@ -122,9 +119,7 @@ func runCommand(args []string, stdout, stderr io.Writer) int {
|
||||
}
|
||||
|
||||
llmClient, err := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{
|
||||
BaseURL: cfg.llmBaseURL,
|
||||
Model: cfg.model,
|
||||
Timeout: cfg.timeout,
|
||||
Timeout: defaults.LLMRequestTimeoutDefault,
|
||||
})
|
||||
if err != nil {
|
||||
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
||||
@@ -184,9 +179,7 @@ func serveCommand(args []string, stderr io.Writer) int {
|
||||
}
|
||||
|
||||
llmClient, err := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{
|
||||
BaseURL: cfg.llmBaseURL,
|
||||
Model: cfg.model,
|
||||
Timeout: cfg.timeout,
|
||||
Timeout: defaults.LLMRequestTimeoutDefault,
|
||||
})
|
||||
if err != nil {
|
||||
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
||||
@@ -285,9 +278,6 @@ func parseServeArgs(args []string) (*serveConfig, error) {
|
||||
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")
|
||||
fs.DurationVar(&cfg.timeout, "timeout", defaults.LLMRequestTimeoutDefault, "LLM request timeout")
|
||||
|
||||
if err := fs.Parse(args); err != nil {
|
||||
return nil, err
|
||||
@@ -397,5 +387,5 @@ func printSummary(stderr io.Writer, res *domain.RunResult) {
|
||||
func printUsage(w io.Writer) {
|
||||
fmt.Fprintln(w, "usage: scriptorium <run|serve> ...")
|
||||
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.Fprintf(w, " serve: scriptorium serve --addr %s --prompt-dir DIR --profile-dir DIR [--llm-base-url URL] [--schema-dir DIR] [--model NAME] [--timeout 10m]\n", defaults.HTTPAddrDefault)
|
||||
fmt.Fprintf(w, " serve: scriptorium serve --addr %s --prompt-dir DIR --profile-dir DIR [--schema-dir DIR]\n", defaults.HTTPAddrDefault)
|
||||
}
|
||||
|
||||
@@ -155,8 +155,24 @@ func TestParseServeArgsRequiredFlags(t *testing.T) {
|
||||
if cfg.addr != defaults.HTTPAddrDefault {
|
||||
t.Fatalf("expected default addr %s, got %q", defaults.HTTPAddrDefault, cfg.addr)
|
||||
}
|
||||
if cfg.timeout != defaults.LLMRequestTimeoutDefault {
|
||||
t.Fatalf("expected default timeout %s, got %s", defaults.LLMRequestTimeoutDefault, cfg.timeout)
|
||||
if cfg.schemaDir != defaults.SchemaDirDefault {
|
||||
t.Fatalf("expected default schema dir %q, got %q", defaults.SchemaDirDefault, cfg.schemaDir)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseServeArgsRejectsRuntimeOverrideFlags(t *testing.T) {
|
||||
base := []string{"--prompt-dir", "./prompts", "--profile-dir", "./profiles"}
|
||||
tests := [][]string{
|
||||
{"--llm-base-url", "http://localhost:8000/v1"},
|
||||
{"--model", "gpt-4o-mini"},
|
||||
{"--timeout", "30s"},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
_, err := parseServeArgs(append(base, tc...))
|
||||
if err == nil {
|
||||
t.Fatalf("expected unknown flag error for %q", tc[0])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -213,7 +229,7 @@ func TestRunCommandVarsOptional(t *testing.T) {
|
||||
"--profile-dir", "./profiles",
|
||||
"--prompt", "p",
|
||||
"--input", "transcript=./t.md",
|
||||
"--llm-base-url", "://bad-url",
|
||||
"--llm-base-url", "http://[::1",
|
||||
"--model", "m",
|
||||
}, &stdout, &stderr)
|
||||
|
||||
@@ -223,8 +239,8 @@ func TestRunCommandVarsOptional(t *testing.T) {
|
||||
if strings.Contains(stderr.String(), "var parse error") {
|
||||
t.Fatalf("expected --var to be optional, got stderr=%q", stderr.String())
|
||||
}
|
||||
if !strings.Contains(stderr.String(), "llm client error") {
|
||||
t.Fatalf("expected llm client error after parsing succeeds, got stderr=%q", stderr.String())
|
||||
if !strings.Contains(stderr.String(), "llm client error") && !strings.Contains(stderr.String(), "run error") {
|
||||
t.Fatalf("expected post-parse execution error, got stderr=%q", stderr.String())
|
||||
}
|
||||
if stdout.Len() != 0 {
|
||||
t.Fatalf("expected no stdout output on error, got %q", stdout.String())
|
||||
|
||||
@@ -5,12 +5,13 @@ import (
|
||||
)
|
||||
|
||||
type runRequestDTO struct {
|
||||
PromptID string `json:"prompt_id"`
|
||||
PromptVersion string `json:"prompt_version,omitempty"`
|
||||
ProfileID string `json:"profile_id,omitempty"`
|
||||
Inputs map[string]inputRefDTO `json:"inputs"`
|
||||
Vars map[string]string `json:"vars,omitempty"`
|
||||
Model *modelOverrideRequestDTO `json:"model,omitempty"`
|
||||
PromptID string `json:"prompt_id"`
|
||||
PromptVersion string `json:"prompt_version,omitempty"`
|
||||
ProfileID string `json:"profile_id,omitempty"`
|
||||
Inputs map[string]inputRefDTO `json:"inputs"`
|
||||
Vars map[string]string `json:"vars,omitempty"`
|
||||
Model *modelOverrideRequestDTO `json:"model,omitempty"`
|
||||
IncludeRawOutput bool `json:"include_raw_output,omitempty"`
|
||||
}
|
||||
|
||||
type inputRefDTO struct {
|
||||
@@ -35,7 +36,7 @@ type runResponseDTO struct {
|
||||
Artifact artifactDTO `json:"artifact"`
|
||||
Validation validationDTO `json:"validation"`
|
||||
Metadata metadataDTO `json:"metadata"`
|
||||
RawModelOutput string `json:"raw_model_output"`
|
||||
RawModelOutput *string `json:"raw_model_output,omitempty"`
|
||||
}
|
||||
|
||||
type artifactDTO struct {
|
||||
|
||||
@@ -90,7 +90,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, runResponseDTO{
|
||||
resp := runResponseDTO{
|
||||
Artifact: artifactDTO{
|
||||
Name: res.Artifact.Name,
|
||||
ContentType: res.Artifact.ContentType,
|
||||
@@ -133,8 +133,12 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
ValidationStatus: string(res.Validation.Status),
|
||||
RepairAttemptsUsed: res.Validation.RepairAttempts,
|
||||
},
|
||||
RawModelOutput: res.RawOutput,
|
||||
})
|
||||
}
|
||||
if req.IncludeRawOutput {
|
||||
raw := res.RawOutput
|
||||
resp.RawModelOutput = &raw
|
||||
}
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
func mapValidation(v domain.ValidationResult) validationDTO {
|
||||
|
||||
@@ -117,6 +117,9 @@ func TestHandlerPostRunsSuccessWithExplicitProfileID(t *testing.T) {
|
||||
if strings.Contains(w.Body.String(), secret) {
|
||||
t.Fatalf("response leaked raw API key value: %s", w.Body.String())
|
||||
}
|
||||
if _, ok := resp["raw_model_output"]; ok {
|
||||
t.Fatalf("expected raw_model_output to be omitted by default, got %#v", resp["raw_model_output"])
|
||||
}
|
||||
|
||||
if r.last.PromptID != "prompt-1" {
|
||||
t.Fatalf("expected request prompt_id prompt-1, got %q", r.last.PromptID)
|
||||
@@ -265,7 +268,7 @@ func TestHandlerRawAPIKeyRejectedByStrictJSON(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandlerValidationFailureStillSuccess(t *testing.T) {
|
||||
func TestHandlerValidationFailureStillSuccessAndRawOutputOptIn(t *testing.T) {
|
||||
h := NewHandler(&fakeRunner{result: &domain.RunResult{
|
||||
Artifact: domain.Artifact{Body: []byte("bad json")},
|
||||
RawOutput: "bad json",
|
||||
@@ -292,6 +295,24 @@ func TestHandlerValidationFailureStillSuccess(t *testing.T) {
|
||||
if status, ok := validation["status"].(string); !ok || status != "failed" {
|
||||
t.Fatalf("expected validation status=failed, got %#v", validation["status"])
|
||||
}
|
||||
if _, ok := resp["raw_model_output"]; ok {
|
||||
t.Fatalf("expected raw_model_output omitted by default, got %#v", resp["raw_model_output"])
|
||||
}
|
||||
|
||||
reqWithRaw := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}},"include_raw_output":true}`))
|
||||
wWithRaw := httptest.NewRecorder()
|
||||
h.ServeHTTP(wWithRaw, reqWithRaw)
|
||||
|
||||
if wWithRaw.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", wWithRaw.Code, wWithRaw.Body.String())
|
||||
}
|
||||
var respWithRaw map[string]any
|
||||
if err := json.Unmarshal(wWithRaw.Body.Bytes(), &respWithRaw); err != nil {
|
||||
t.Fatalf("invalid JSON response: %v", err)
|
||||
}
|
||||
if got, ok := respWithRaw["raw_model_output"].(string); !ok || got != "bad json" {
|
||||
t.Fatalf("expected raw_model_output to be included when requested, got %#v", respWithRaw["raw_model_output"])
|
||||
}
|
||||
}
|
||||
|
||||
func wrap(stage error, cause error) error {
|
||||
|
||||
@@ -41,6 +41,12 @@ func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
|
||||
if len(p.Templates) != 2 {
|
||||
t.Fatalf("expected 2 messages, got %d", len(p.Templates))
|
||||
}
|
||||
if len(p.Inputs) != 1 {
|
||||
t.Fatalf("expected 1 input, got %d", len(p.Inputs))
|
||||
}
|
||||
if p.Inputs[0].ContentType != "text/markdown" {
|
||||
t.Fatalf("expected input content_type to be preserved, got %q", p.Inputs[0].ContentType)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("valid file-backed prompt", func(t *testing.T) {
|
||||
@@ -70,6 +76,12 @@ func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
|
||||
if p.DefaultProfile != "local-default" {
|
||||
t.Fatalf("unexpected default profile: %q", p.DefaultProfile)
|
||||
}
|
||||
if len(p.Inputs) != 1 {
|
||||
t.Fatalf("expected one input, got %d", len(p.Inputs))
|
||||
}
|
||||
if p.Inputs[0].ContentType != "" {
|
||||
t.Fatalf("expected missing content_type to remain empty, got %q", p.Inputs[0].ContentType)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("version lookup", func(t *testing.T) {
|
||||
@@ -94,6 +106,7 @@ func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
|
||||
{name: "duplicate input names", id: "duplicate_input_names", targetErr: ErrInvalidPromptDefinition, errSubstrs: []string{"duplicate input name"}},
|
||||
{name: "invalid validation mode", id: "invalid_validation_mode", targetErr: ErrInvalidPromptDefinition, errSubstrs: []string{"invalid validation mode"}},
|
||||
{name: "json_schema without schema_path", id: "json_schema_without_schema_path", targetErr: ErrInvalidPromptDefinition, errSubstrs: []string{"schema_path"}},
|
||||
{name: "unknown input field", id: "unknown_input_field", targetErr: ErrInvalidYAML, errSubstrs: []string{"field unknown_input_setting not found"}},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
13
internal/promptdef/testdata/unknown_input_field.yaml
vendored
Normal file
13
internal/promptdef/testdata/unknown_input_field.yaml
vendored
Normal file
@@ -0,0 +1,13 @@
|
||||
id: unknown-input-field
|
||||
version: "1.0.0"
|
||||
inputs:
|
||||
- name: transcript
|
||||
required: true
|
||||
unknown_input_setting: true
|
||||
messages:
|
||||
- role: user
|
||||
content: "Hi"
|
||||
output:
|
||||
format: text
|
||||
validation_mode: none
|
||||
repair_attempts: 0
|
||||
@@ -393,6 +393,55 @@ func TestRunnerRunAPIKeyEnvMissingEnvironmentValueFailsClearly(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunnerRunRuntimeAPIKeyEnvOverrideWorks(t *testing.T) {
|
||||
const envName = "SCRIPTORIUM_RUNTIME_API_KEY"
|
||||
t.Setenv(envName, "runtime-secret")
|
||||
|
||||
promptRepo := &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)}
|
||||
execRepo := &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{
|
||||
"exec": {ID: "exec", Endpoint: "http://profile/v1", Model: "profile-model"},
|
||||
}}
|
||||
runner := NewRunner(promptRepo, execRepo, defaultArtifactReader(), defaultRenderer(), &fakeLLM{resp: &domain.GenerateResponse{Content: "ok"}}, nil)
|
||||
|
||||
res, err := runner.Run(context.Background(), domain.RunRequest{
|
||||
PromptID: "p",
|
||||
ProfileID: "exec",
|
||||
Inputs: singleInputRef(),
|
||||
Execution: &domain.ExecutionTarget{APIKeyEnv: envName},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("expected no error, got %v", err)
|
||||
}
|
||||
if res.EffectiveModelParams.APIKeyEnv != envName {
|
||||
t.Fatalf("expected runtime api_key_env override in effective params, got %q", res.EffectiveModelParams.APIKeyEnv)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunnerRunRuntimeAPIKeyEnvOverrideBeatsProfile(t *testing.T) {
|
||||
const profileEnv = "SCRIPTORIUM_PROFILE_API_KEY"
|
||||
const runtimeEnv = "SCRIPTORIUM_RUNTIME_API_KEY"
|
||||
t.Setenv(runtimeEnv, "runtime-secret")
|
||||
|
||||
promptRepo := &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)}
|
||||
execRepo := &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{
|
||||
"exec": {ID: "exec", Endpoint: "http://profile/v1", Model: "profile-model", APIKeyEnv: profileEnv},
|
||||
}}
|
||||
runner := NewRunner(promptRepo, execRepo, defaultArtifactReader(), defaultRenderer(), &fakeLLM{resp: &domain.GenerateResponse{Content: "ok"}}, nil)
|
||||
|
||||
res, err := runner.Run(context.Background(), domain.RunRequest{
|
||||
PromptID: "p",
|
||||
ProfileID: "exec",
|
||||
Inputs: singleInputRef(),
|
||||
Execution: &domain.ExecutionTarget{APIKeyEnv: runtimeEnv},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("expected no error, got %v", err)
|
||||
}
|
||||
if res.EffectiveModelParams.APIKeyEnv != runtimeEnv {
|
||||
t.Fatalf("expected runtime override to beat profile api_key_env, got %q", res.EffectiveModelParams.APIKeyEnv)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunnerRunAPIKeyValueNotPresentInMetadata(t *testing.T) {
|
||||
const envName = "SCRIPTORIUM_TEST_API_KEY"
|
||||
const secret = "top-secret-value"
|
||||
|
||||
Reference in New Issue
Block a user