From 5328f04f6db90db3c88ee517a2811207042c7bf2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 6 May 2026 15:15:52 +0000 Subject: [PATCH] Add modular PreparedRun text/json formatters with role-grouped text output and secret-safety tests --- internal/format/prepared_run.go | 40 ++++++-- internal/format/prepared_run_test.go | 144 +++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 10 deletions(-) diff --git a/internal/format/prepared_run.go b/internal/format/prepared_run.go index d276a6c..4bfa8d5 100644 --- a/internal/format/prepared_run.go +++ b/internal/format/prepared_run.go @@ -64,6 +64,15 @@ func FormatPreparedRun(prepared *domain.PreparedRun, outputFormat PreparedRunOut return formatter.Format(prepared) } +// FormatPreparedRunByName parses a format name and formats a prepared run. +func FormatPreparedRunByName(prepared *domain.PreparedRun, rawFormat string) ([]byte, error) { + outputFormat, err := ParsePreparedRunOutputFormat(rawFormat) + if err != nil { + return nil, err + } + return FormatPreparedRun(prepared, outputFormat) +} + type jsonPreparedRunFormatter struct{} func (jsonPreparedRunFormatter) Format(prepared *domain.PreparedRun) ([]byte, error) { @@ -126,17 +135,28 @@ func (textPreparedRunFormatter) Format(prepared *domain.PreparedRun) ([]byte, er } fmt.Fprintln(&b, "messages:") - for i, msg := range prepared.Messages { - fmt.Fprintf(&b, " - index: %d\n", i) - fmt.Fprintf(&b, " role: %s\n", msg.Role) - fmt.Fprintln(&b, " content: |") - content := msg.Content - if content == "" { - fmt.Fprintln(&b, " ") - continue + roleOrder := make([]string, 0) + byRole := make(map[string][]domain.RenderedMessage) + for _, msg := range prepared.Messages { + if _, exists := byRole[msg.Role]; !exists { + roleOrder = append(roleOrder, msg.Role) } - for _, line := range strings.Split(content, "\n") { - fmt.Fprintf(&b, " %s\n", line) + byRole[msg.Role] = append(byRole[msg.Role], msg) + } + for _, role := range roleOrder { + fmt.Fprintf(&b, " %s:\n", role) + messages := byRole[role] + for i, msg := range messages { + fmt.Fprintf(&b, " - message: %d\n", i+1) + fmt.Fprintln(&b, " content: |") + content := msg.Content + if content == "" { + fmt.Fprintln(&b, " ") + continue + } + for _, line := range strings.Split(content, "\n") { + fmt.Fprintf(&b, " %s\n", line) + } } } diff --git a/internal/format/prepared_run_test.go b/internal/format/prepared_run_test.go index 048e93f..a9687d3 100644 --- a/internal/format/prepared_run_test.go +++ b/internal/format/prepared_run_test.go @@ -1,10 +1,115 @@ package format import ( + "encoding/json" "errors" + "strings" "testing" + + "gitea.maximumdirect.net/eric/scriptorium/internal/domain" ) +func TestTextFormatterIncludesPreparedRunDetails(t *testing.T) { + prepared := samplePreparedRun() + + out, err := FormatPreparedRun(prepared, PreparedRunFormatText) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + s := string(out) + + for _, want := range []string{ + "prompt: prompt.id", + "prompt_version: v1", + "selected_profile_id: local-fast", + "endpoint: http://llm/v1", + "model: gpt-test", + "temperature: 0.4", + "max_tokens: 256", + "top_p: 0.8", + "timeout_seconds: 45", + "reasoning_effort: medium", + "api_key_env: SCRIPTORIUM_API_KEY", + "prompt_hash: prompt-hash", + "rendered_prompt_hash: rendered-hash", + "glossary: hash-glossary", + "transcript: hash-transcript", + "messages:", + " system:", + " user:", + "System guidance.", + "Summarize the transcript.", + "Include key entities.", + "Second user message.", + } { + if !strings.Contains(s, want) { + t.Fatalf("expected text output to include %q, got:\n%s", want, s) + } + } +} + +func TestTextFormatterDoesNotIncludeResolvedAPIKeyValue(t *testing.T) { + const secret = "super-secret-api-key" + t.Setenv("SCRIPTORIUM_API_KEY", secret) + + out, err := FormatPreparedRun(samplePreparedRun(), PreparedRunFormatText) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if strings.Contains(string(out), secret) { + t.Fatalf("text output should not include resolved api key value: %s", out) + } +} + +func TestJSONFormatterEmitsValidJSONAndIncludesPreparedRunFields(t *testing.T) { + prepared := samplePreparedRun() + + out, err := FormatPreparedRun(prepared, PreparedRunFormatJSON) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + var decoded map[string]any + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("expected valid json output, got %v", err) + } + + if decoded["prompt_id"] != "prompt.id" { + t.Fatalf("expected prompt_id in json output, got %#v", decoded["prompt_id"]) + } + if decoded["prompt_version"] != "v1" { + t.Fatalf("expected prompt_version in json output, got %#v", decoded["prompt_version"]) + } + if decoded["selected_profile_id"] != "local-fast" { + t.Fatalf("expected selected_profile_id in json output, got %#v", decoded["selected_profile_id"]) + } + if decoded["rendered_prompt_hash"] != "rendered-hash" { + t.Fatalf("expected rendered_prompt_hash in json output, got %#v", decoded["rendered_prompt_hash"]) + } + if _, ok := decoded["effective_model_params"]; !ok { + t.Fatalf("expected effective_model_params in json output, got %#v", decoded) + } + if _, ok := decoded["input_hashes"]; !ok { + t.Fatalf("expected input_hashes in json output, got %#v", decoded) + } + if _, ok := decoded["messages"]; !ok { + t.Fatalf("expected messages in json output, got %#v", decoded) + } +} + +func TestJSONFormatterDoesNotIncludeResolvedAPIKeyValue(t *testing.T) { + const secret = "super-secret-api-key" + t.Setenv("SCRIPTORIUM_API_KEY", secret) + + out, err := FormatPreparedRun(samplePreparedRun(), PreparedRunFormatJSON) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if strings.Contains(string(out), secret) { + t.Fatalf("json output should not include resolved api key value: %s", out) + } +} + func TestParsePreparedRunOutputFormatRecognizesSupportedNames(t *testing.T) { tests := []struct { name string @@ -39,3 +144,42 @@ func TestParsePreparedRunOutputFormatUnknownFails(t *testing.T) { t.Fatalf("expected ErrUnknownPreparedRunFormat, got %v", err) } } + +func TestFormatPreparedRunByNameUnknownFailsClearly(t *testing.T) { + _, err := FormatPreparedRunByName(samplePreparedRun(), "yaml") + if err == nil { + t.Fatal("expected unknown format error") + } + if !errors.Is(err, ErrUnknownPreparedRunFormat) { + t.Fatalf("expected ErrUnknownPreparedRunFormat, got %v", err) + } +} + +func samplePreparedRun() *domain.PreparedRun { + return &domain.PreparedRun{ + PromptID: "prompt.id", + PromptVersion: "v1", + PromptHash: "prompt-hash", + SelectedProfileID: "local-fast", + EffectiveModelParams: domain.ExecutionTarget{ + Endpoint: "http://llm/v1", + Model: "gpt-test", + Temperature: 0.4, + MaxTokens: 256, + TopP: 0.8, + TimeoutSeconds: 45, + ReasoningEffort: "medium", + APIKeyEnv: "SCRIPTORIUM_API_KEY", + }, + InputHashes: map[string]string{ + "transcript": "hash-transcript", + "glossary": "hash-glossary", + }, + RenderedPromptHash: "rendered-hash", + Messages: []domain.RenderedMessage{ + {Role: "system", Content: "System guidance."}, + {Role: "user", Content: "Summarize the transcript.\nInclude key entities."}, + {Role: "user", Content: "Second user message."}, + }, + } +}