From 4d646f161adf12998df0d5e5101133c62e301db6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 6 May 2026 21:36:30 +0000 Subject: [PATCH] Review Scriptorium integration architecture --- internal/app/run.go | 2 +- internal/stage/analyze.go | 7 +++--- internal/stage/analyze_test.go | 44 ++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/internal/app/run.go b/internal/app/run.go index 6558305..a82faf9 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -9,7 +9,7 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/config" ) -// Run executes the placeholder stage pipeline and persists manifest state. +// Run executes the pipeline plan and persists manifest state. func Run(ctx context.Context, args []string, out io.Writer) error { fs := flag.NewFlagSet("run", flag.ContinueOnError) fs.SetOutput(io.Discard) diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index 35abc09..59bf305 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -24,7 +24,6 @@ func (analyzeStage) Declares() IODecl { return IODecl{ Inputs: []artifacts.Ref{ {Kind: "transcript_processed", Category: "transcripts", RelativePath: "transcripts/processed.json"}, - {Kind: "artifact", Category: "artifacts", RelativePath: "artifacts/session_recap.md"}, }, Outputs: []artifacts.Ref{ {Kind: "session_recap", Category: "artifacts", RelativePath: "artifacts/session_recap.md"}, @@ -177,7 +176,7 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S 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 { + if err := requireNonEmptyFile(finalRenderOutputPath, artifactName+" render output"); err != nil { return nil, fmt.Errorf("analyze: %w", err) } if err := validateJSONFile(finalRenderOutputPath); err != nil { @@ -238,7 +237,7 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S } finalOutputPath := coalesceString(res.OutputPath, req.OutputPath) - if err := requireNonEmptyFile(finalOutputPath, "session recap output"); err != nil { + if err := requireNonEmptyFile(finalOutputPath, artifactName+" output"); err != nil { return nil, fmt.Errorf("analyze: %w", err) } @@ -382,7 +381,7 @@ func resolveInputPathForRead(paths artifacts.SessionPaths, sessionDir, pathValue func resolveScriptoriumOutputPath(paths artifacts.SessionPaths, configured string) (string, error) { outputPath := strings.TrimSpace(configured) if outputPath == "" { - outputPath = "artifacts/session_recap.md" + return "", fmt.Errorf("scriptorium artifact output path is required") } if filepath.IsAbs(outputPath) { return filepath.Clean(outputPath), nil diff --git a/internal/stage/analyze_test.go b/internal/stage/analyze_test.go index fedfaf4..1c4213f 100644 --- a/internal/stage/analyze_test.go +++ b/internal/stage/analyze_test.go @@ -84,6 +84,29 @@ func TestAnalyzeRenderDebugFalseDoesNotCallRenderArtifact(t *testing.T) { } } +func TestAnalyzeRenderDebugArtifactOverrideFalseWinsOverGlobalTrue(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 + artifact := env.Config.Pipeline.Scriptorium.Artifacts["session_recap"] + disabled := false + artifact.RenderDebug = &disabled + env.Config.Pipeline.Scriptorium.Artifacts["session_recap"] = artifact + + result, 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)) + } + if result.Metadata["render_debug_enabled"] != false { + t.Fatalf("render_debug_enabled = %#v, want false", result.Metadata["render_debug_enabled"]) + } +} + func TestAnalyzeRenderDebugTrueCallsRenderBeforeRun(t *testing.T) { env, m, _ := setupAnalyzeEnv(t) paths := env.ArtifactStore.SessionPaths(m.SessionID) @@ -365,6 +388,27 @@ func TestAnalyzeFailsWhenRequiredPreviousRecapMissing(t *testing.T) { } } +func TestAnalyzeFailsWhenOutputPathMissing(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := env.ArtifactStore.SessionPaths(m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) + + artifact := env.Config.Pipeline.Scriptorium.Artifacts["session_recap"] + artifact.OutputPath = "" + env.Config.Pipeline.Scriptorium.Artifacts["session_recap"] = artifact + + _, err := (analyzeStage{}).Run(context.Background(), env, m) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "output path") { + t.Fatalf("error = %q, want output path context", err.Error()) + } + if len(fake.RunRequests) != 0 { + t.Fatalf("run requests = %d, want 0 when output path resolution fails", len(fake.RunRequests)) + } +} + func TestAnalyzeFailsWhenProcessedTranscriptMissing(t *testing.T) { env, m, _ := setupAnalyzeEnv(t) _, err := (analyzeStage{}).Run(context.Background(), env, m)