Relaxed CLI requirements when defaults are specified in the profile or application defaults
This commit is contained in:
@@ -41,6 +41,11 @@ type runConfig struct {
|
||||
maxTokens int
|
||||
schemaDir string
|
||||
timeout time.Duration
|
||||
|
||||
llmBaseURLSet bool
|
||||
modelSet bool
|
||||
temperatureSet bool
|
||||
maxTokensSet bool
|
||||
}
|
||||
|
||||
type serveConfig struct {
|
||||
@@ -127,16 +132,21 @@ func runCommand(args []string, stdout, stderr io.Writer) int {
|
||||
validate.NewStandardValidator(cfg.schemaDir),
|
||||
)
|
||||
|
||||
res, runErr := runner.Run(context.Background(), domain.RunRequest{
|
||||
ProfileID: cfg.profileID,
|
||||
Inputs: inputs,
|
||||
Vars: varMappings,
|
||||
Model: &domain.ModelTarget{
|
||||
var modelOverride *domain.ModelTarget
|
||||
if cfg.llmBaseURLSet || cfg.modelSet || cfg.temperatureSet || cfg.maxTokensSet {
|
||||
modelOverride = &domain.ModelTarget{
|
||||
Endpoint: cfg.llmBaseURL,
|
||||
Model: cfg.model,
|
||||
Temperature: cfg.temperature,
|
||||
MaxTokens: cfg.maxTokens,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
res, runErr := runner.Run(context.Background(), domain.RunRequest{
|
||||
ProfileID: cfg.profileID,
|
||||
Inputs: inputs,
|
||||
Vars: varMappings,
|
||||
Model: modelOverride,
|
||||
})
|
||||
if runErr != nil {
|
||||
fmt.Fprintf(stderr, "run error: %v\n", runErr)
|
||||
@@ -227,18 +237,15 @@ func parseRunArgs(args []string) (*runConfig, error) {
|
||||
if len(cfg.inputRaw) == 0 {
|
||||
return nil, errors.New("at least one --input is required")
|
||||
}
|
||||
if strings.TrimSpace(cfg.llmBaseURL) == "" {
|
||||
return nil, errors.New("--llm-base-url is required")
|
||||
}
|
||||
if strings.TrimSpace(cfg.model) == "" {
|
||||
return nil, errors.New("--model is required")
|
||||
}
|
||||
|
||||
cfg.profileDir = filepath.Clean(cfg.profileDir)
|
||||
cfg.schemaDir = filepath.Clean(cfg.schemaDir)
|
||||
if cfg.outputPath != "" {
|
||||
cfg.outputPath = filepath.Clean(cfg.outputPath)
|
||||
}
|
||||
cfg.llmBaseURLSet = flagWasSet(fs, "llm-base-url")
|
||||
cfg.modelSet = flagWasSet(fs, "model")
|
||||
cfg.temperatureSet = flagWasSet(fs, "temperature")
|
||||
cfg.maxTokensSet = flagWasSet(fs, "max-tokens")
|
||||
|
||||
return cfg, nil
|
||||
}
|
||||
@@ -312,6 +319,16 @@ func parseMapping(value string) (string, string, error) {
|
||||
return key, val, nil
|
||||
}
|
||||
|
||||
func flagWasSet(fs *flag.FlagSet, name string) bool {
|
||||
set := false
|
||||
fs.Visit(func(f *flag.Flag) {
|
||||
if f.Name == name {
|
||||
set = true
|
||||
}
|
||||
})
|
||||
return set
|
||||
}
|
||||
|
||||
func writeOutput(stdout io.Writer, outputPath string, body []byte) error {
|
||||
if outputPath == "" {
|
||||
_, err := stdout.Write(body)
|
||||
@@ -351,6 +368,6 @@ 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 --profile-dir DIR --profile-id ID --input name=path [--input ...] --llm-base-url URL --model NAME [--var k=v] [--out path] [--timeout 10m]")
|
||||
fmt.Fprintln(w, " run: scriptorium run --profile-dir DIR --profile-id ID --input name=path [--input ...] [--llm-base-url URL] [--model NAME] [--var k=v] [--out path] [--timeout 10m]")
|
||||
fmt.Fprintln(w, " serve: scriptorium serve --addr :8080 --profile-dir DIR --llm-base-url URL [--schema-dir DIR] [--llm-api-key KEY] [--model NAME] [--timeout 10m]")
|
||||
}
|
||||
|
||||
@@ -65,15 +65,19 @@ func TestParseRunArgsRequiredFlags(t *testing.T) {
|
||||
if err == nil {
|
||||
t.Fatal("expected missing --input error")
|
||||
}
|
||||
}
|
||||
|
||||
_, err = parseRunArgs([]string{"--profile-dir", "./profiles", "--profile-id", "p", "--input", "a=b", "--model", "m"})
|
||||
if err == nil {
|
||||
t.Fatal("expected missing --llm-base-url error")
|
||||
func TestParseRunArgsAllowsOmittedModelAndBaseURL(t *testing.T) {
|
||||
cfg, err := parseRunArgs([]string{
|
||||
"--profile-dir", "./profiles",
|
||||
"--profile-id", "p",
|
||||
"--input", "a=b",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("expected valid args without model/base url, got %v", err)
|
||||
}
|
||||
|
||||
_, err = parseRunArgs([]string{"--profile-dir", "./profiles", "--profile-id", "p", "--input", "a=b", "--llm-base-url", "http://x/v1"})
|
||||
if err == nil {
|
||||
t.Fatal("expected missing --model error")
|
||||
if cfg.llmBaseURL != "" || cfg.model != "" {
|
||||
t.Fatalf("expected empty model/baseurl, got model=%q base=%q", cfg.model, cfg.llmBaseURL)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,4 +171,31 @@ func TestRunCommandVarsOptional(t *testing.T) {
|
||||
if !strings.Contains(stderr.String(), "llm client error") {
|
||||
t.Fatalf("expected llm client error after parsing succeeds, got stderr=%q", stderr.String())
|
||||
}
|
||||
if stdout.Len() != 0 {
|
||||
t.Fatalf("expected no stdout output on error, got %q", stdout.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteOutputAndSummaryUseSeparateWriters(t *testing.T) {
|
||||
var stdout bytes.Buffer
|
||||
var stderr bytes.Buffer
|
||||
|
||||
if err := writeOutput(&stdout, "", []byte("artifact-body")); err != nil {
|
||||
t.Fatalf("unexpected writeOutput error: %v", err)
|
||||
}
|
||||
printSummary(&stderr, &domain.RunResult{
|
||||
ProfileID: "p",
|
||||
ProfileVersion: "1",
|
||||
ModelName: "m",
|
||||
Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic},
|
||||
PromptHash: "h",
|
||||
InputHashes: map[string]string{"in": "x"},
|
||||
})
|
||||
|
||||
if stdout.String() != "artifact-body" {
|
||||
t.Fatalf("expected artifact output on stdout, got %q", stdout.String())
|
||||
}
|
||||
if !strings.Contains(stderr.String(), "profile=p@1") {
|
||||
t.Fatalf("expected summary on stderr, got %q", stderr.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,15 +44,31 @@ type artifactDTO struct {
|
||||
}
|
||||
|
||||
type metadataDTO struct {
|
||||
ProfileID string `json:"profile_id"`
|
||||
ProfileVersion string `json:"profile_version"`
|
||||
ModelName string `json:"model_name"`
|
||||
Endpoint string `json:"endpoint"`
|
||||
InputHashes map[string]string `json:"input_hashes"`
|
||||
PromptHash string `json:"prompt_hash"`
|
||||
Usage tokenUsageDTO `json:"usage"`
|
||||
StartTime time.Time `json:"start_time"`
|
||||
EndTime time.Time `json:"end_time"`
|
||||
RunID string `json:"run_id"`
|
||||
ProfileID string `json:"profile_id"`
|
||||
ProfileVersion string `json:"profile_version"`
|
||||
ProfileHash string `json:"profile_hash"`
|
||||
ModelName string `json:"model_name"`
|
||||
Endpoint string `json:"endpoint"`
|
||||
ModelParams modelParamsDTO `json:"model_params"`
|
||||
InputHashes map[string]string `json:"input_hashes"`
|
||||
PromptHash string `json:"prompt_hash"`
|
||||
Usage tokenUsageDTO `json:"usage"`
|
||||
StartTime time.Time `json:"start_time"`
|
||||
EndTime time.Time `json:"end_time"`
|
||||
DurationMS int64 `json:"duration_ms"`
|
||||
ValidationMode string `json:"validation_mode"`
|
||||
ValidationStatus string `json:"validation_status"`
|
||||
RepairAttemptsUsed int `json:"repair_attempts_used"`
|
||||
}
|
||||
|
||||
type modelParamsDTO struct {
|
||||
Endpoint string `json:"endpoint"`
|
||||
Model string `json:"model"`
|
||||
Temperature float64 `json:"temperature"`
|
||||
MaxTokens int `json:"max_tokens"`
|
||||
TopP float64 `json:"top_p"`
|
||||
TimeoutSeconds int `json:"timeout_seconds"`
|
||||
}
|
||||
|
||||
type tokenUsageDTO struct {
|
||||
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
@@ -37,7 +36,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
var req runRequestDTO
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "invalid_json", fmt.Sprintf("invalid JSON request: %v", err))
|
||||
writeError(w, http.StatusBadRequest, "invalid_json", "invalid JSON request body")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -79,8 +78,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
Model: model,
|
||||
})
|
||||
if err != nil {
|
||||
status, code := mapRunError(err)
|
||||
writeError(w, status, code, err.Error())
|
||||
status, code, message := mapRunError(err)
|
||||
writeError(w, status, code, message)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -95,19 +94,33 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
},
|
||||
Validation: mapValidation(res.Validation),
|
||||
Metadata: metadataDTO{
|
||||
RunID: res.RunID,
|
||||
ProfileID: res.ProfileID,
|
||||
ProfileVersion: res.ProfileVersion,
|
||||
ProfileHash: res.ProfileHash,
|
||||
ModelName: res.ModelName,
|
||||
Endpoint: res.Endpoint,
|
||||
InputHashes: res.InputHashes,
|
||||
PromptHash: res.PromptHash,
|
||||
ModelParams: modelParamsDTO{
|
||||
Endpoint: res.ModelParams.Endpoint,
|
||||
Model: res.ModelParams.Model,
|
||||
Temperature: res.ModelParams.Temperature,
|
||||
MaxTokens: res.ModelParams.MaxTokens,
|
||||
TopP: res.ModelParams.TopP,
|
||||
TimeoutSeconds: res.ModelParams.TimeoutSeconds,
|
||||
},
|
||||
InputHashes: res.InputHashes,
|
||||
PromptHash: res.PromptHash,
|
||||
Usage: tokenUsageDTO{
|
||||
PromptTokens: res.Usage.PromptTokens,
|
||||
CompletionTokens: res.Usage.CompletionTokens,
|
||||
TotalTokens: res.Usage.TotalTokens,
|
||||
},
|
||||
StartTime: res.StartTime,
|
||||
EndTime: res.EndTime,
|
||||
StartTime: res.StartTime,
|
||||
EndTime: res.EndTime,
|
||||
DurationMS: res.Duration.Milliseconds(),
|
||||
ValidationMode: string(res.Validation.Mode),
|
||||
ValidationStatus: string(res.Validation.Status),
|
||||
RepairAttemptsUsed: res.Validation.RepairAttempts,
|
||||
},
|
||||
RawModelOutput: res.RawOutput,
|
||||
})
|
||||
@@ -124,24 +137,24 @@ func mapValidation(v domain.ValidationResult) validationDTO {
|
||||
}
|
||||
}
|
||||
|
||||
func mapRunError(err error) (int, string) {
|
||||
func mapRunError(err error) (int, string, string) {
|
||||
switch {
|
||||
case errors.Is(err, profile.ErrProfileNotFound):
|
||||
return http.StatusNotFound, "profile_not_found"
|
||||
return http.StatusNotFound, "profile_not_found", "profile not found"
|
||||
case errors.Is(err, usecase.ErrInvalidRequest):
|
||||
return http.StatusBadRequest, "invalid_request"
|
||||
return http.StatusBadRequest, "invalid_request", "invalid run request"
|
||||
case errors.Is(err, usecase.ErrProfileLoad):
|
||||
return http.StatusBadRequest, "profile_load_failed"
|
||||
return http.StatusBadRequest, "profile_load_failed", "failed to load profile"
|
||||
case errors.Is(err, usecase.ErrArtifactLoad):
|
||||
return http.StatusBadRequest, "artifact_read_failed"
|
||||
return http.StatusBadRequest, "artifact_read_failed", "failed to read input artifact"
|
||||
case errors.Is(err, usecase.ErrPromptRender):
|
||||
return http.StatusBadRequest, "prompt_render_failed"
|
||||
return http.StatusBadRequest, "prompt_render_failed", "failed to render prompt"
|
||||
case errors.Is(err, usecase.ErrLLMGenerate):
|
||||
return http.StatusBadGateway, "llm_failed"
|
||||
return http.StatusBadGateway, "llm_failed", "model generation request failed"
|
||||
case errors.Is(err, usecase.ErrValidation):
|
||||
return http.StatusInternalServerError, "validation_runtime_failed"
|
||||
return http.StatusInternalServerError, "validation_runtime_failed", "validation runtime failed"
|
||||
default:
|
||||
return http.StatusInternalServerError, "internal_error"
|
||||
return http.StatusInternalServerError, "internal_error", "internal server error"
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -33,6 +34,7 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
|
||||
start := time.Now().UTC()
|
||||
end := start.Add(2 * time.Second)
|
||||
r := &fakeRunner{result: &domain.RunResult{
|
||||
RunID: "11111111-1111-4111-8111-111111111111",
|
||||
Artifact: domain.Artifact{
|
||||
Name: "output",
|
||||
ContentType: "text/plain",
|
||||
@@ -43,14 +45,24 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
|
||||
Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true},
|
||||
ProfileID: "p1",
|
||||
ProfileVersion: "1.0.0",
|
||||
ProfileHash: "phash",
|
||||
ModelName: "m1",
|
||||
Endpoint: "http://llm/v1",
|
||||
InputHashes: map[string]string{"transcript": "h1"},
|
||||
PromptHash: "ph",
|
||||
Usage: domain.TokenUsage{PromptTokens: 1, CompletionTokens: 2, TotalTokens: 3},
|
||||
StartTime: start,
|
||||
EndTime: end,
|
||||
RawOutput: "hello",
|
||||
ModelParams: domain.ModelTarget{
|
||||
Endpoint: "http://llm/v1",
|
||||
Model: "m1",
|
||||
Temperature: 0.2,
|
||||
MaxTokens: 42,
|
||||
TopP: 0.9,
|
||||
TimeoutSeconds: 120,
|
||||
},
|
||||
InputHashes: map[string]string{"transcript": "h1"},
|
||||
PromptHash: "ph",
|
||||
Usage: domain.TokenUsage{PromptTokens: 1, CompletionTokens: 2, TotalTokens: 3},
|
||||
StartTime: start,
|
||||
EndTime: end,
|
||||
Duration: 2 * time.Second,
|
||||
RawOutput: "hello",
|
||||
}}
|
||||
|
||||
h := NewHandler(r)
|
||||
@@ -86,10 +98,26 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
|
||||
t.Fatalf("expected validation.status passed, got %#v", validation["status"])
|
||||
}
|
||||
metadata := resp["metadata"].(map[string]any)
|
||||
if metadata["run_id"] != "11111111-1111-4111-8111-111111111111" {
|
||||
t.Fatalf("unexpected metadata.run_id: %#v", metadata["run_id"])
|
||||
}
|
||||
if metadata["profile_hash"] != "phash" {
|
||||
t.Fatalf("unexpected metadata.profile_hash: %#v", metadata["profile_hash"])
|
||||
}
|
||||
usage := metadata["usage"].(map[string]any)
|
||||
if usage["total_tokens"] != float64(3) {
|
||||
t.Fatalf("expected usage.total_tokens=3, got %#v", usage["total_tokens"])
|
||||
}
|
||||
if metadata["duration_ms"] != float64(2000) {
|
||||
t.Fatalf("expected duration_ms=2000, got %#v", metadata["duration_ms"])
|
||||
}
|
||||
if metadata["validation_mode"] != "basic" || metadata["validation_status"] != "passed" {
|
||||
t.Fatalf("unexpected validation metadata: mode=%#v status=%#v", metadata["validation_mode"], metadata["validation_status"])
|
||||
}
|
||||
modelParams := metadata["model_params"].(map[string]any)
|
||||
if modelParams["model"] != "m1" {
|
||||
t.Fatalf("unexpected model_params.model: %#v", modelParams["model"])
|
||||
}
|
||||
if resp["raw_model_output"] != "hello" {
|
||||
t.Fatalf("expected raw model output hello, got %#v", resp["raw_model_output"])
|
||||
}
|
||||
@@ -153,6 +181,20 @@ func TestHandlerUsecaseErrorMapping(t *testing.T) {
|
||||
if w.Code != tc.status {
|
||||
t.Fatalf("expected %d, got %d body=%s", tc.status, w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON response: %v", err)
|
||||
}
|
||||
errBody := resp["error"].(map[string]any)
|
||||
if _, ok := errBody["code"].(string); !ok {
|
||||
t.Fatalf("expected error code string, got %#v", errBody["code"])
|
||||
}
|
||||
if msg, ok := errBody["message"].(string); !ok || msg == "" {
|
||||
t.Fatalf("expected non-empty error message, got %#v", errBody["message"])
|
||||
}
|
||||
if strings.Contains(w.Body.String(), "read failed") || strings.Contains(w.Body.String(), "render failed") || strings.Contains(w.Body.String(), "llm failed") {
|
||||
t.Fatalf("expected response to avoid leaking internal cause details, got %s", w.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user