From 4aafeef830974bfedc1921afda3df5f81a16ce84 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 6 May 2026 21:30:19 +0000 Subject: [PATCH] Add Scriptorium render diagnostics --- internal/config/config.go | 15 ++- internal/config/scriptorium_test.go | 24 ++++ internal/stage/analyze.go | 142 +++++++++++++++----- internal/stage/analyze_test.go | 195 ++++++++++++++++++++++++++++ 4 files changed, 338 insertions(+), 38 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 5b696eb..384eb3f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -94,13 +94,14 @@ type ScriptoriumConfig struct { // ScriptoriumArtifactConfig configures one named output artifact workflow. type ScriptoriumArtifactConfig struct { - Enabled bool `yaml:"enabled"` - PromptID string `yaml:"prompt_id"` - ProfileID string `yaml:"profile_id"` - OutputPath string `yaml:"output_path"` - Timeout string `yaml:"timeout"` - Inputs map[string]ScriptoriumInputConfig `yaml:"inputs"` - Vars map[string]any `yaml:"vars"` + Enabled bool `yaml:"enabled"` + RenderDebug *bool `yaml:"render_debug"` + PromptID string `yaml:"prompt_id"` + ProfileID string `yaml:"profile_id"` + OutputPath string `yaml:"output_path"` + Timeout string `yaml:"timeout"` + Inputs map[string]ScriptoriumInputConfig `yaml:"inputs"` + Vars map[string]any `yaml:"vars"` } // ScriptoriumInputConfig configures one named prompt input source. diff --git a/internal/config/scriptorium_test.go b/internal/config/scriptorium_test.go index 7141b59..5876955 100644 --- a/internal/config/scriptorium_test.go +++ b/internal/config/scriptorium_test.go @@ -108,6 +108,30 @@ func TestScriptoriumLoadAndValidate(t *testing.T) { output_kind: session_recap `, }, + { + name: "artifact render_debug override is accepted", + scriptoriumYAML: `scriptorium: + binary: scriptorium + render_debug: false + artifacts: + session_recap: + enabled: true + render_debug: true + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md + inputs: + transcript: + source: processed_transcript + required: true +`, + assert: func(t *testing.T, cfg *Config) { + t.Helper() + artifact := cfg.Pipeline.Scriptorium.Artifacts["session_recap"] + if artifact.RenderDebug == nil || *artifact.RenderDebug != true { + t.Fatalf("artifact render_debug = %#v, want true", artifact.RenderDebug) + } + }, + }, { name: "multiple artifact definitions can be decoded", scriptoriumYAML: `scriptorium: diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index c0d4ee9..35abc09 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -2,6 +2,7 @@ package stage import ( "context" + "encoding/json" "fmt" "os" "path/filepath" @@ -130,6 +131,79 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S return nil, fmt.Errorf("analyze: resolve timeout: %w", err) } + logPaths := []string{} + generatedConfigs := []string{} + meta := map[string]any{ + "stage": "analyze", + "artifact_name": artifactName, + "prompt_id": artifactCfg.PromptID, + "profile_id": artifactCfg.ProfileID, + "binary": env.Config.Pipeline.Scriptorium.Binary, + "config_path": env.Config.Pipeline.Scriptorium.ConfigPath, + "input_paths": inputPaths, + "input_names": sortedMapKeys(inputPaths), + "omitted_optional_inputs": omittedOptionalInputs, + "vars": vars, + "timeout": timeout.String(), + "processed_transcript_path": processedTranscriptPath, + "processed_transcript_source": processedSource, + "render_debug_enabled": resolveRenderDebugEnabled(env.Config.Pipeline.Scriptorium.RenderDebug, artifactCfg.RenderDebug), + } + + if meta["render_debug_enabled"] == true { + renderOutputPath := filepath.Join(paths.ArtifactsDir, artifactName+".render.json") + renderStdoutPath := filepath.Join(paths.LogsDir, "scriptorium."+artifactName+".render.stdout.log") + renderStderrPath := filepath.Join(paths.LogsDir, "scriptorium."+artifactName+".render.stderr.log") + renderGeneratedConfigPath := filepath.Join(paths.ConfigDir, "scriptorium."+artifactName+".render.generated.yml") + + renderReq := scriptorium.RenderArtifactRequest{ + Binary: env.Config.Pipeline.Scriptorium.Binary, + ConfigPath: env.Config.Pipeline.Scriptorium.ConfigPath, + PromptID: artifactCfg.PromptID, + ProfileID: artifactCfg.ProfileID, + InputPaths: inputPaths, + Vars: vars, + OutputPath: renderOutputPath, + StdoutLogPath: renderStdoutPath, + StderrLogPath: renderStderrPath, + GeneratedConfigPath: renderGeneratedConfigPath, + Timeout: timeout, + } + renderRes, renderErr := env.Scriptorium.RenderArtifact(ctx, renderReq) + if renderErr != nil { + return nil, fmt.Errorf("analyze: scriptorium render failed: %w", renderErr) + } + if renderRes.ValidationFailed { + return nil, fmt.Errorf("analyze: scriptorium render returned validation_failed=true") + } + finalRenderOutputPath := coalesceString(renderRes.OutputPath, renderReq.OutputPath) + if err := requireNonEmptyFile(finalRenderOutputPath, "session recap render output"); err != nil { + return nil, fmt.Errorf("analyze: %w", err) + } + if err := validateJSONFile(finalRenderOutputPath); err != nil { + return nil, fmt.Errorf("analyze: render diagnostics %q invalid json: %w", finalRenderOutputPath, err) + } + + logPaths = append(logPaths, renderStdoutPath, renderStderrPath) + generatedConfigs = append(generatedConfigs, renderGeneratedConfigPath) + meta["render_output_path"] = finalRenderOutputPath + meta["render_stdout_log_path"] = renderStdoutPath + meta["render_stderr_log_path"] = renderStderrPath + meta["render_generated_config_path"] = renderGeneratedConfigPath + meta["render_adapter_exit_code"] = renderRes.ExitCode + meta["render_adapter_duration_ms"] = renderRes.Duration.Milliseconds() + meta["render_adapter_command_mode"] = renderRes.CommandMode + meta["render_adapter_prompt_id"] = renderRes.PromptID + meta["render_adapter_profile_id"] = renderRes.ProfileID + meta["render_adapter_output_path"] = renderRes.OutputPath + meta["render_adapter_generated_config"] = renderRes.GeneratedConfigPath + meta["render_adapter_stdout_log_path"] = renderRes.StdoutLogPath + meta["render_adapter_stderr_log_path"] = renderRes.StderrLogPath + if renderRes.Metadata != nil { + meta["render_adapter_metadata"] = renderRes.Metadata + } + } + req := scriptorium.RunArtifactRequest{ Binary: env.Config.Pipeline.Scriptorium.Binary, ConfigPath: env.Config.Pipeline.Scriptorium.ConfigPath, @@ -175,43 +249,30 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S AbsolutePath: finalOutputPath, } - meta := map[string]any{ - "stage": "analyze", - "artifact_name": artifactName, - "prompt_id": artifactCfg.PromptID, - "profile_id": artifactCfg.ProfileID, - "output_path": finalOutputPath, - "generated_config_path": generatedConfigPath, - "stdout_log_path": stdoutLogPath, - "stderr_log_path": stderrLogPath, - "input_paths": inputPaths, - "input_names": sortedMapKeys(inputPaths), - "omitted_optional_inputs": omittedOptionalInputs, - "vars": vars, - "timeout": timeout.String(), - "binary": env.Config.Pipeline.Scriptorium.Binary, - "config_path": env.Config.Pipeline.Scriptorium.ConfigPath, - "processed_transcript_path": processedTranscriptPath, - "processed_transcript_source": processedSource, - "adapter_exit_code": res.ExitCode, - "adapter_duration_ms": res.Duration.Milliseconds(), - "adapter_command_mode": res.CommandMode, - "adapter_prompt_id": res.PromptID, - "adapter_profile_id": res.ProfileID, - "adapter_validation_failed": res.ValidationFailed, - "adapter_output_path": res.OutputPath, - "adapter_generated_config": res.GeneratedConfigPath, - "adapter_stdout_log_path": res.StdoutLogPath, - "adapter_stderr_log_path": res.StderrLogPath, - } + logPaths = append(logPaths, stdoutLogPath, stderrLogPath) + generatedConfigs = append(generatedConfigs, generatedConfigPath) + meta["output_path"] = finalOutputPath + meta["generated_config_path"] = generatedConfigPath + meta["stdout_log_path"] = stdoutLogPath + meta["stderr_log_path"] = stderrLogPath + meta["adapter_exit_code"] = res.ExitCode + meta["adapter_duration_ms"] = res.Duration.Milliseconds() + meta["adapter_command_mode"] = res.CommandMode + meta["adapter_prompt_id"] = res.PromptID + meta["adapter_profile_id"] = res.ProfileID + meta["adapter_validation_failed"] = res.ValidationFailed + meta["adapter_output_path"] = res.OutputPath + meta["adapter_generated_config"] = res.GeneratedConfigPath + meta["adapter_stdout_log_path"] = res.StdoutLogPath + meta["adapter_stderr_log_path"] = res.StderrLogPath if res.Metadata != nil { meta["adapter_metadata"] = res.Metadata } return &StageResult{ Outputs: []artifacts.Ref{artifactRef}, - Logs: []string{stdoutLogPath, stderrLogPath}, - GeneratedConfigs: []string{generatedConfigPath}, + Logs: logPaths, + GeneratedConfigs: generatedConfigs, Metadata: meta, }, nil } @@ -460,3 +521,22 @@ func coalesceString(primary, fallback string) string { } return fallback } + +func resolveRenderDebugEnabled(global bool, perArtifact *bool) bool { + if perArtifact == nil { + return global + } + return *perArtifact +} + +func validateJSONFile(path string) error { + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read file: %w", err) + } + var payload any + if err := json.Unmarshal(data, &payload); err != nil { + return fmt.Errorf("decode json: %w", err) + } + return nil +} diff --git a/internal/stage/analyze_test.go b/internal/stage/analyze_test.go index 7986b20..fedfaf4 100644 --- a/internal/stage/analyze_test.go +++ b/internal/stage/analyze_test.go @@ -68,6 +68,133 @@ func TestAnalyzeGeneratesSessionRecapFromProcessedTranscript(t *testing.T) { } } +func TestAnalyzeRenderDebugFalseDoesNotCallRenderArtifact(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + + env.Config.Pipeline.Scriptorium.RenderDebug = false + + _, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(fake.RenderRequests) != 0 { + t.Fatalf("render requests = %d, want 0", len(fake.RenderRequests)) + } +} + +func TestAnalyzeRenderDebugTrueCallsRenderBeforeRun(t *testing.T) { + env, m, _ := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + + env.Config.Pipeline.Scriptorium.RenderDebug = true + runner := &orderedScriptoriumRunner{ + RenderBody: `{"rendered":true}`, + RunBody: "session recap\n", + } + env.Scriptorium = runner + + _, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if got := strings.Join(runner.Calls, ","); got != "render,run" { + t.Fatalf("call order = %q, want render,run", got) + } +} + +func TestAnalyzeRenderOutputPathIsRecorded(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + env.Config.Pipeline.Scriptorium.RenderDebug = true + + result, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(fake.RenderRequests) != 1 { + t.Fatalf("render requests = %d, want 1", len(fake.RenderRequests)) + } + if result.Metadata["render_output_path"] != filepath.Join(paths.ArtifactsDir, "session_recap.render.json") { + t.Fatalf("render_output_path = %#v, want session_recap.render.json path", result.Metadata["render_output_path"]) + } + if len(result.Logs) != 4 { + t.Fatalf("logs = %#v, want render+run stdout/stderr logs", result.Logs) + } + if len(result.GeneratedConfigs) != 2 { + t.Fatalf("generated configs = %#v, want render+run generated configs", result.GeneratedConfigs) + } +} + +func TestAnalyzeRenderFailurePreventsRun(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + env.Config.Pipeline.Scriptorium.RenderDebug = true + fake.RenderErr = errors.New("render boom") + + _, err := (analyzeStage{}).Run(context.Background(), env, m) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "scriptorium render failed") { + t.Fatalf("error = %q, want render failure context", err.Error()) + } + if len(fake.RenderRequests) != 1 { + t.Fatalf("render requests = %d, want 1", len(fake.RenderRequests)) + } + if len(fake.RunRequests) != 0 { + t.Fatalf("run requests = %d, want 0 when render fails", len(fake.RunRequests)) + } +} + +func TestAnalyzeRenderInvalidJSONFailsClearly(t *testing.T) { + env, m, _ := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + env.Config.Pipeline.Scriptorium.RenderDebug = true + runner := &orderedScriptoriumRunner{ + RenderBody: `not-json`, + RunBody: "session recap\n", + } + env.Scriptorium = runner + + _, err := (analyzeStage{}).Run(context.Background(), env, m) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "render diagnostics") { + t.Fatalf("error = %q, want render diagnostics context", err.Error()) + } + if got := strings.Join(runner.Calls, ","); got != "render" { + t.Fatalf("call order = %q, want only render before failure", got) + } +} + +func TestAnalyzeRunStillSucceedsWhenRenderSucceeds(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + env.Config.Pipeline.Scriptorium.RenderDebug = true + + result, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(fake.RenderRequests) != 1 || len(fake.RunRequests) != 1 { + t.Fatalf("render requests = %d, run requests = %d, want 1/1", len(fake.RenderRequests), len(fake.RunRequests)) + } + if result.Metadata["render_debug_enabled"] != true { + t.Fatalf("render_debug_enabled = %#v, want true", result.Metadata["render_debug_enabled"]) + } + if err := requireNonEmptyFile(filepath.Join(paths.ArtifactsDir, "session_recap.md"), "session recap output"); err != nil { + t.Fatalf("session recap output validation failed: %v", err) + } +} + func TestAnalyzeOmitsOptionalPreviousRecapWhenUnavailable(t *testing.T) { env, m, fake := setupAnalyzeEnv(t) paths := env.ArtifactStore.SessionPaths(m.SessionID) @@ -106,6 +233,66 @@ func TestAnalyzeOmitsOptionalPreviousRecapWhenUnavailable(t *testing.T) { } } +type orderedScriptoriumRunner struct { + Calls []string + RenderErr error + RunErr error + RenderBody string + RunBody string +} + +func (r *orderedScriptoriumRunner) RenderArtifact(_ context.Context, req scriptorium.RenderArtifactRequest) (scriptorium.ArtifactResult, error) { + r.Calls = append(r.Calls, "render") + if r.RenderErr != nil { + return scriptorium.ArtifactResult{}, r.RenderErr + } + body := r.RenderBody + if strings.TrimSpace(body) == "" { + body = `{"rendered":true}` + } + writeAnalyzeFileNoTest(req.OutputPath, body) + writeAnalyzeFileNoTest(req.StdoutLogPath, "render stdout\n") + writeAnalyzeFileNoTest(req.StderrLogPath, "render stderr\n") + if req.GeneratedConfigPath != "" { + writeAnalyzeFileNoTest(req.GeneratedConfigPath, "schema: scriptorium.generated.v1\n") + } + return scriptorium.ArtifactResult{ + OutputPath: req.OutputPath, + StdoutLogPath: req.StdoutLogPath, + StderrLogPath: req.StderrLogPath, + GeneratedConfigPath: req.GeneratedConfigPath, + CommandMode: scriptorium.CommandModeRender, + PromptID: req.PromptID, + ProfileID: req.ProfileID, + }, nil +} + +func (r *orderedScriptoriumRunner) RunArtifact(_ context.Context, req scriptorium.RunArtifactRequest) (scriptorium.ArtifactResult, error) { + r.Calls = append(r.Calls, "run") + if r.RunErr != nil { + return scriptorium.ArtifactResult{}, r.RunErr + } + body := r.RunBody + if strings.TrimSpace(body) == "" { + body = "session recap\n" + } + writeAnalyzeFileNoTest(req.OutputPath, body) + writeAnalyzeFileNoTest(req.StdoutLogPath, "run stdout\n") + writeAnalyzeFileNoTest(req.StderrLogPath, "run stderr\n") + if req.GeneratedConfigPath != "" { + writeAnalyzeFileNoTest(req.GeneratedConfigPath, "schema: scriptorium.generated.v1\n") + } + return scriptorium.ArtifactResult{ + OutputPath: req.OutputPath, + StdoutLogPath: req.StdoutLogPath, + StderrLogPath: req.StderrLogPath, + GeneratedConfigPath: req.GeneratedConfigPath, + CommandMode: scriptorium.CommandModeRun, + PromptID: req.PromptID, + ProfileID: req.ProfileID, + }, nil +} + func TestAnalyzeIncludesPreviousRecapWhenConfiguredAndAvailable(t *testing.T) { env, m, fake := setupAnalyzeEnv(t) paths := env.ArtifactStore.SessionPaths(m.SessionID) @@ -377,3 +564,11 @@ func writeAnalyzeFile(t *testing.T, path, contents string) { t.Fatalf("WriteFile(%q): %v", path, err) } } + +func writeAnalyzeFileNoTest(path, contents string) { + if strings.TrimSpace(path) == "" { + return + } + _ = os.MkdirAll(filepath.Dir(path), 0o755) + _ = os.WriteFile(path, []byte(contents), 0o644) +}