diff --git a/docs/internal/app-orchestration.md b/docs/internal/app-orchestration.md index f2b799a..53984a1 100644 --- a/docs/internal/app-orchestration.md +++ b/docs/internal/app-orchestration.md @@ -94,12 +94,6 @@ Single-report generation shares this setup: For `scriptorium_markdown` reports, generation then: 12. Runs Scriptorium report generation to the managed report path. -13. Copies the managed report to the requested `--out` path when provided. -14. Saves metadata with the managed report path. -15. If distributor notification is enabled, notifies using the managed report - path as the source file. -16. Saves a distributor notification debug artifact and updates metadata with - its path. For `generated_text_template` reports, generation then: @@ -110,13 +104,17 @@ For `generated_text_template` reports, generation then: 15. Validates and saves normalized generated text. 16. Builds and saves a typed render context. 17. Renders Markdown from the embedded template to the managed report path. -18. Saves final metadata with generated-text paths, render context path, schema - ID, and managed report path. -19. Copies the managed report to the requested `--out` path when provided. -20. If distributor notification is enabled, notifies using the managed report - path as the source file. -21. Saves a distributor notification debug artifact and updates metadata with - its path. + +After either mode has produced a managed Markdown report, shared finalization: + +1. Copies the managed report to the requested `--out` or `--out-dir` path when + provided. +2. Saves final metadata with the managed report path and any generated-text + artifact paths already produced. +3. If distributor notification is enabled, notifies using the managed report + path as the source file. +4. Saves a distributor notification debug artifact and updates metadata with + its path. If render preflight returns both a result and an error, preflight JSON and metadata are persisted before the error is returned. If Scriptorium report diff --git a/internal/app/app.go b/internal/app/app.go index 77c7683..7b11d53 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -584,32 +584,16 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro DataPackagePath: dataPackagePath, OutputPath: reportPath, }) - if runErr == nil && req.OutputPath != "" && req.OutputPath != reportPath { - if err := fileutil.CopyFileAtomic(reportPath, req.OutputPath); err != nil { - return nil, err - } - } - outputPath := reportPath - if req.OutputPath != "" { - outputPath = req.OutputPath - } - metadata.RenderedReportPath = reportPath - metadataPath, metadataErr = store.SaveMetadata(ctx, metadata) - if metadataErr != nil { - return nil, metadataErr - } - if runErr != nil { - return nil, runErr - } - - notification, notificationPath, err := notifyReport(ctx, req.Config, req.Resolved, reportPath, metadata, req.Notifier, store) - if notificationPath != "" { - metadata.NotificationPath = notificationPath - metadataPath, metadataErr = store.SaveMetadata(ctx, metadata) - if metadataErr != nil { - return nil, metadataErr - } - } + finalized, err := finalizeRenderedReport(ctx, finalizeRenderedReportRequest{ + Config: req.Config, + Store: store, + Resolved: req.Resolved, + Metadata: metadata, + ManagedReportPath: reportPath, + OutputPath: req.OutputPath, + Notifier: req.Notifier, + GenerationErr: runErr, + }) if err != nil { return nil, err } @@ -621,15 +605,15 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro DataPackagePath: dataPackagePath, PreflightPath: preflightPath, ReportPath: reportPath, - OutputPath: outputPath, - NotificationPath: notificationPath, - Metadata: metadata, - MetadataPath: metadataPath, + OutputPath: finalized.OutputPath, + NotificationPath: finalized.NotificationPath, + Metadata: finalized.Metadata, + MetadataPath: finalized.MetadataPath, PriorSnapshot: priorSnapshot, RecentChanges: recentChanges, RenderResult: renderResult, RunResult: runResult, - Notification: notification, + Notification: finalized.Notification, }, nil } @@ -723,32 +707,18 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest) if err := fileutil.WriteFileAtomic(reportPath, rendered); err != nil { return nil, err } - req.metadata.RenderedReportPath = reportPath - metadataPath, err := req.store.SaveMetadata(ctx, req.metadata) + finalized, err := finalizeRenderedReport(ctx, finalizeRenderedReportRequest{ + Config: req.Config, + Store: req.store, + Resolved: req.Resolved, + Metadata: req.metadata, + ManagedReportPath: reportPath, + OutputPath: req.OutputPath, + Notifier: req.Notifier, + }) if err != nil { return nil, err } - outputPath := reportPath - if req.OutputPath != "" { - outputPath = req.OutputPath - if req.OutputPath != reportPath { - if err := fileutil.CopyFileAtomic(reportPath, req.OutputPath); err != nil { - return nil, err - } - } - } - - notification, notificationPath, notificationErr := notifyReport(ctx, req.Config, req.Resolved, reportPath, req.metadata, req.Notifier, req.store) - if notificationPath != "" { - req.metadata.NotificationPath = notificationPath - metadataPath, err = req.store.SaveMetadata(ctx, req.metadata) - if err != nil { - return nil, err - } - } - if notificationErr != nil { - return nil, notificationErr - } return &ReportResult{ ModuleSnapshot: req.moduleSnapshot, @@ -757,10 +727,10 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest) DataPackagePath: req.dataPackagePath, PreflightPath: req.preflightPath, ReportPath: reportPath, - OutputPath: outputPath, - NotificationPath: notificationPath, - Metadata: req.metadata, - MetadataPath: metadataPath, + OutputPath: finalized.OutputPath, + NotificationPath: finalized.NotificationPath, + Metadata: finalized.Metadata, + MetadataPath: finalized.MetadataPath, PriorSnapshot: req.priorSnapshot, RecentChanges: req.recentChanges, RenderResult: req.renderResult, @@ -769,10 +739,80 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest) GeneratedTextResultPath: generatedTextResultPath, GeneratedTextPath: generatedTextPath, RenderContextPath: renderContextPath, - Notification: notification, + Notification: finalized.Notification, }, nil } +type finalizeRenderedReportRequest struct { + Config config.Config + Store state.Store + Resolved report.Resolved + Metadata state.Metadata + ManagedReportPath string + OutputPath string + Notifier Notifier + GenerationErr error +} + +type finalizeRenderedReportResult struct { + OutputPath string + NotificationPath string + Metadata state.Metadata + MetadataPath string + Notification *NotificationResult +} + +func finalizeRenderedReport(ctx context.Context, req finalizeRenderedReportRequest) (finalizeRenderedReportResult, error) { + if req.Store == nil { + return finalizeRenderedReportResult{}, fmt.Errorf("state store is required") + } + if req.ManagedReportPath == "" { + return finalizeRenderedReportResult{}, fmt.Errorf("managed report path is required for report %q", req.Resolved.Definition.ID) + } + + metadata := req.Metadata + metadata.RenderedReportPath = req.ManagedReportPath + outputPath := req.ManagedReportPath + if req.OutputPath != "" { + outputPath = req.OutputPath + if req.GenerationErr == nil && req.OutputPath != req.ManagedReportPath { + if err := fileutil.CopyFileAtomic(req.ManagedReportPath, req.OutputPath); err != nil { + return finalizeRenderedReportResult{}, err + } + } + } + + metadataPath, err := req.Store.SaveMetadata(ctx, metadata) + if err != nil { + return finalizeRenderedReportResult{}, err + } + result := finalizeRenderedReportResult{ + OutputPath: outputPath, + Metadata: metadata, + MetadataPath: metadataPath, + } + if req.GenerationErr != nil { + return result, req.GenerationErr + } + + notification, notificationPath, err := notifyReport(ctx, req.Config, req.Resolved, req.ManagedReportPath, metadata, req.Notifier, req.Store) + if notificationPath != "" { + metadata.NotificationPath = notificationPath + metadataPath, saveErr := req.Store.SaveMetadata(ctx, metadata) + if saveErr != nil { + return finalizeRenderedReportResult{}, saveErr + } + result.Metadata = metadata + result.MetadataPath = metadataPath + result.NotificationPath = notificationPath + } + result.Notification = notification + if err != nil { + return result, err + } + return result, nil +} + func notifyReport(ctx context.Context, cfg config.Config, resolved report.Resolved, reportPath string, metadata state.Metadata, notifier Notifier, store state.Store) (*NotificationResult, string, error) { notifier, enabled := reportNotifier(cfg, notifier) if !enabled { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 3370e2b..eab6538 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -645,6 +645,73 @@ func TestGenerateHourlyReportNotificationFailureFailsReport(t *testing.T) { } } +func TestGenerateReportSavesFinalMetadataForMarkdownAndGeneratedTextReports(t *testing.T) { + t.Run("Markdown", func(t *testing.T) { + server := dailyBundleServer(t) + cfg := dailyTestConfig(t, server) + cfg.Workspace.Root = t.TempDir() + resolved, err := ResolveGenerate(GenerateRequest{ + Config: cfg, + Report: ReportDaily, + Date: mustParse("2026-05-29T12:00:00-05:00"), + }, mustParse("2026-05-29T05:00:00-05:00")) + if err != nil { + t.Fatalf("ResolveGenerate() error = %v", err) + } + + result, err := GenerateReport(context.Background(), ReportRequest{ + Config: cfg, + Resolved: resolved, + Renderer: successfulRenderer("# Daily Report\n"), + }) + if err != nil { + t.Fatalf("GenerateReport() error = %v", err) + } + + saved := readMetadataForTest(t, result.MetadataPath) + if saved.RenderedReportPath != result.ReportPath || saved.NotificationPath != "" { + t.Fatalf("saved metadata = %#v, want final rendered path without notification", saved) + } + if saved.GeneratedTextSchemaID != "" || saved.GeneratedTextPath != "" || saved.RenderContextPath != "" { + t.Fatalf("saved markdown metadata has generated-text fields: %#v", saved) + } + }) + + t.Run("GeneratedTextTemplate", func(t *testing.T) { + server := hourlyBundleServer(t) + cfg := hourlyGeneratedTextConfig(t, server) + cfg.Notify.Distributor.Enabled = false + resolved, store, _, _ := resolveHourlyGeneratedTextFixture(t, cfg) + renderer := &recordingRenderer{ + renderResult: &scriptorium.RenderResult{ExitCode: 0}, + structuredRunResult: &scriptorium.StructuredRunResult{ExitCode: 0}, + structuredRunBody: validHourlyGeneratedTextJSON(), + } + + result, err := GenerateReport(context.Background(), ReportRequest{ + Config: cfg, + Resolved: resolved, + Renderer: renderer, + Store: store, + }) + if err != nil { + t.Fatalf("GenerateReport() error = %v", err) + } + + saved := readMetadataForTest(t, result.MetadataPath) + if saved.RenderedReportPath != result.ReportPath || saved.NotificationPath != "" { + t.Fatalf("saved metadata = %#v, want final rendered path without notification", saved) + } + if saved.GeneratedTextSchemaID != "hourly" || + saved.GeneratedTextRawPath != result.GeneratedTextRawPath || + saved.GeneratedTextResultPath != result.GeneratedTextResultPath || + saved.GeneratedTextPath != result.GeneratedTextPath || + saved.RenderContextPath != result.RenderContextPath { + t.Fatalf("saved generated-text metadata = %#v, want generated-text artifact links", saved) + } + }) +} + func TestGenerateTomorrowReportNotificationUsesTomorrowTemplateValues(t *testing.T) { server := dailyBundleServer(t) cfg := config.Defaults() @@ -2065,6 +2132,28 @@ func TestRunBatchContinuesAfterNotificationFailure(t *testing.T) { if !failedThreeDay { t.Fatalf("reports = %#v, want notification failure on 3-day item", result.Reports) } + + cfg.Workspace.Root = t.TempDir() + err = RunBatch(context.Background(), BatchRequest{ + Config: cfg, + Batch: BatchMorning, + Now: mustParse("2026-05-29T05:00:00-05:00"), + Renderer: &selectiveRenderer{ + runBody: "# Batch Report\n", + }, + Notifier: &recordingNotifier{ + errByReport: map[report.ID]error{ + report.ThreeDay: errors.New("distributor unavailable"), + }, + }, + }) + var batchErr BatchError + if !errors.As(err, &batchErr) { + t.Fatalf("RunBatch() error = %T %v, want BatchError", err, err) + } + if batchErr.Result == nil || batchErr.Result.Failed != 1 || batchErr.Result.Succeeded != 2 { + t.Fatalf("RunBatch() result = %#v, want notification failure aggregate", batchErr.Result) + } } func TestRunBatchUsesOutputDirectory(t *testing.T) { @@ -2095,6 +2184,17 @@ func TestRunBatchUsesOutputDirectory(t *testing.T) { if _, err := os.Stat(want); err != nil { t.Fatalf("expected output copy %q: %v", want, err) } + reportData, err := os.ReadFile(result.Reports[0].ReportPath) + if err != nil { + t.Fatalf("read managed report: %v", err) + } + copyData, err := os.ReadFile(want) + if err != nil { + t.Fatalf("read output copy: %v", err) + } + if string(copyData) != string(reportData) { + t.Fatalf("batch output copy differs from managed report") + } } func TestBatchOutputPathUsesHourlyOutputName(t *testing.T) { @@ -2286,6 +2386,19 @@ func hourlyArtifactPaths(t *testing.T, store state.Store, resolved report.Resolv return paths } +func readMetadataForTest(t *testing.T, path string) state.Metadata { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read metadata %q: %v", path, err) + } + var metadata state.Metadata + if err := json.Unmarshal(data, &metadata); err != nil { + t.Fatalf("decode metadata %q: %v", path, err) + } + return metadata +} + func assertGeneratedReportError(t *testing.T, err error, resolved report.Resolved, operation string) { t.Helper() if err == nil {