diff --git a/docs/internal/app-orchestration.md b/docs/internal/app-orchestration.md index 0f436db..8865cb1 100644 --- a/docs/internal/app-orchestration.md +++ b/docs/internal/app-orchestration.md @@ -84,7 +84,9 @@ defaults. Single-report commands validate the report command, collect once through `internal/collect`, resolve the requested report, and pass the resolved report -plus explicit collection into `GenerateReport`. +plus explicit collection into `GenerateReport`. `GenerateDetailed` returns the +resulting `ReportResult`; `Generate` wraps the same workflow for error-only +callers. `GenerateReport` then uses this setup: @@ -191,7 +193,9 @@ inspection view. - Generated-text report errors preserve available intermediate artifacts and do not create extra output copies. - Single-report notification errors are wrapped with report ID, RunID, and - managed report path context. + managed report path context. Detailed generation returns the inspectable + report, metadata, and notification artifact paths when finalization has + already saved them. - Batch notification errors are recorded on the top-level batch notification result and do not change individual report item status. - Metadata and artifact path errors include filesystem context. diff --git a/internal/app/app.go b/internal/app/app.go index 117524b..4a06bcc 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -268,29 +268,33 @@ func (e *NotificationError) Unwrap() error { } func Generate(ctx context.Context, req GenerateRequest) error { + _, err := GenerateDetailed(ctx, req) + return err +} + +func GenerateDetailed(ctx context.Context, req GenerateRequest) (*ReportResult, error) { now := req.Now if now.IsZero() { now = time.Now() } collection, err := collectWeather(ctx, req.Config, req.Collector) if err != nil { - return err + return nil, err } resolved, err := ResolveGenerate(req, now) if err != nil { - return err + return nil, err } if resolved.Definition.Generated { - _, err := GenerateReport(ctx, ReportRequest{ + return GenerateReport(ctx, ReportRequest{ Config: req.Config, Resolved: resolved, OutputPath: req.OutputPath, Collection: *collection, Notifier: req.Notifier, }) - return err } - return fmt.Errorf("generate is not implemented") + return nil, fmt.Errorf("generate is not implemented") } func RunBatch(ctx context.Context, req BatchRequest) error { @@ -654,26 +658,37 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro noNotify: req.noNotify, }) if err != nil { - return nil, err + if finalizeResultEmpty(finalized) { + return nil, err + } + return renderedReportResult(reportResultRequest{ + moduleSnapshot: moduleSnapshot, + moduleSnapshotPath: moduleSnapshotPath, + dataPackage: dataPackage, + dataPackagePath: dataPackagePath, + preflightPath: preflightPath, + reportPath: reportPath, + finalized: finalized, + priorSnapshot: priorSnapshot, + recentChanges: recentChanges, + renderResult: renderResult, + runResult: runResult, + }), err } - return &ReportResult{ - ModuleSnapshot: moduleSnapshot, - ModuleSnapshotPath: moduleSnapshotPath, - DataPackage: dataPackage, - DataPackagePath: dataPackagePath, - PreflightPath: preflightPath, - ReportPath: reportPath, - OutputPath: finalized.OutputPath, - NotificationPath: finalized.NotificationPath, - Metadata: finalized.Metadata, - MetadataPath: finalized.MetadataPath, - PriorSnapshot: priorSnapshot, - RecentChanges: recentChanges, - RenderResult: renderResult, - RunResult: runResult, - Notification: finalized.Notification, - }, nil + return renderedReportResult(reportResultRequest{ + moduleSnapshot: moduleSnapshot, + moduleSnapshotPath: moduleSnapshotPath, + dataPackage: dataPackage, + dataPackagePath: dataPackagePath, + preflightPath: preflightPath, + reportPath: reportPath, + finalized: finalized, + priorSnapshot: priorSnapshot, + recentChanges: recentChanges, + renderResult: renderResult, + runResult: runResult, + }), nil } type generatedReportRequest struct { @@ -777,30 +792,97 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest) noNotify: req.noNotify, }) if err != nil { - return nil, err + if finalizeResultEmpty(finalized) { + return nil, err + } + return renderedReportResult(reportResultRequest{ + moduleSnapshot: req.moduleSnapshot, + moduleSnapshotPath: req.moduleSnapshotPath, + dataPackage: req.dataPackage, + dataPackagePath: req.dataPackagePath, + preflightPath: req.preflightPath, + reportPath: reportPath, + finalized: finalized, + priorSnapshot: req.priorSnapshot, + recentChanges: req.recentChanges, + renderResult: req.renderResult, + structuredRunResult: structuredResult, + generatedTextRawPath: req.paths.GeneratedTextRaw, + generatedTextResultPath: generatedTextResultPath, + generatedTextPath: generatedTextPath, + renderContextPath: renderContextPath, + }), err } + return renderedReportResult(reportResultRequest{ + moduleSnapshot: req.moduleSnapshot, + moduleSnapshotPath: req.moduleSnapshotPath, + dataPackage: req.dataPackage, + dataPackagePath: req.dataPackagePath, + preflightPath: req.preflightPath, + reportPath: reportPath, + finalized: finalized, + priorSnapshot: req.priorSnapshot, + recentChanges: req.recentChanges, + renderResult: req.renderResult, + structuredRunResult: structuredResult, + generatedTextRawPath: req.paths.GeneratedTextRaw, + generatedTextResultPath: generatedTextResultPath, + generatedTextPath: generatedTextPath, + renderContextPath: renderContextPath, + }), nil +} + +func finalizeResultEmpty(result finalizeRenderedReportResult) bool { + return result.OutputPath == "" && + result.NotificationPath == "" && + result.MetadataPath == "" && + result.Metadata.RunID == "" && + result.Notification == nil +} + +type reportResultRequest struct { + moduleSnapshot module.Snapshot + moduleSnapshotPath string + dataPackage promptinput.Package + dataPackagePath string + preflightPath string + reportPath string + finalized finalizeRenderedReportResult + priorSnapshot *state.PriorSnapshot + recentChanges []changes.Change + renderResult *scriptorium.RenderResult + runResult *scriptorium.RunResult + structuredRunResult *scriptorium.StructuredRunResult + generatedTextRawPath string + generatedTextResultPath string + generatedTextPath string + renderContextPath string +} + +func renderedReportResult(req reportResultRequest) *ReportResult { return &ReportResult{ ModuleSnapshot: req.moduleSnapshot, ModuleSnapshotPath: req.moduleSnapshotPath, DataPackage: req.dataPackage, DataPackagePath: req.dataPackagePath, PreflightPath: req.preflightPath, - ReportPath: reportPath, - OutputPath: finalized.OutputPath, - NotificationPath: finalized.NotificationPath, - Metadata: finalized.Metadata, - MetadataPath: finalized.MetadataPath, + ReportPath: req.reportPath, + OutputPath: req.finalized.OutputPath, + NotificationPath: req.finalized.NotificationPath, + Metadata: req.finalized.Metadata, + MetadataPath: req.finalized.MetadataPath, PriorSnapshot: req.priorSnapshot, RecentChanges: req.recentChanges, RenderResult: req.renderResult, - StructuredRunResult: structuredResult, - GeneratedTextRawPath: req.paths.GeneratedTextRaw, - GeneratedTextResultPath: generatedTextResultPath, - GeneratedTextPath: generatedTextPath, - RenderContextPath: renderContextPath, - Notification: finalized.Notification, - }, nil + RunResult: req.runResult, + StructuredRunResult: req.structuredRunResult, + GeneratedTextRawPath: req.generatedTextRawPath, + GeneratedTextResultPath: req.generatedTextResultPath, + GeneratedTextPath: req.generatedTextPath, + RenderContextPath: req.renderContextPath, + Notification: req.finalized.Notification, + } } type finalizeRenderedReportRequest struct { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 02c1b11..14b49ac 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -168,6 +168,97 @@ func TestGenerateCollectionFailureStopsBeforeReportExecution(t *testing.T) { } } +func TestGenerateDetailedReturnsReportResult(t *testing.T) { + server := dailyBundleServer(t) + cfg := dailyWorkspaceConfig(t, server) + cfg.Scriptorium.Binary = fakeScriptoriumBinary(t) + collection := collectionForTest(t, cfg) + collector := &recordingCollector{result: &collection} + outputPath := filepath.Join(t.TempDir(), "three-day.md") + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: cfg, + Report: ReportThreeDay, + OutputPath: outputPath, + Now: mustParse("2026-05-29T05:00:00-05:00"), + Collector: collector, + }) + if err != nil { + t.Fatalf("GenerateDetailed() error = %v", err) + } + if result == nil { + t.Fatal("GenerateDetailed() result = nil, want report result") + } + if result.Metadata.ReportID != report.ThreeDay || result.Metadata.RunID == "" { + t.Fatalf("metadata = %#v, want 3-day report metadata with run id", result.Metadata) + } + if result.OutputPath != outputPath { + t.Fatalf("OutputPath = %q, want requested output copy %q", result.OutputPath, outputPath) + } + assertPathsExist(t, result.ModuleSnapshotPath, result.DataPackagePath, result.PreflightPath, result.ReportPath, result.MetadataPath, outputPath) + if len(collector.requests) != 1 { + t.Fatalf("collector requests = %d, want one collection", len(collector.requests)) + } +} + +func TestGenerateReturnsUnderlyingErrorOnly(t *testing.T) { + cfg := config.Defaults() + cfg.WeatherAPI.BaseURL = "" + cfg.Workspace.Root = t.TempDir() + wantErr := errors.New("collector unavailable") + + err := Generate(context.Background(), GenerateRequest{ + Config: cfg, + Report: ReportThreeDay, + Now: mustParse("2026-05-29T05:00:00-05:00"), + Collector: &recordingCollector{err: wantErr}, + }) + if !errors.Is(err, wantErr) { + t.Fatalf("Generate() error = %v, want underlying collector error", err) + } +} + +func TestGenerateDetailedNotificationFailureReturnsInspectableResult(t *testing.T) { + server := hourlyBundleServer(t) + cfg := hourlyGeneratedTextConfig(t, server) + cfg.Scriptorium.Binary = fakeScriptoriumBinary(t) + collection := collectionForTest(t, cfg) + notifier := &recordingNotifier{err: errors.New("upload rejected")} + outputPath := filepath.Join(t.TempDir(), "hourly.md") + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: cfg, + Report: ReportHourly, + OutputPath: outputPath, + Now: mustParse("2026-05-29T08:30:00-05:00"), + Collector: &recordingCollector{result: &collection}, + Notifier: notifier, + }) + if err == nil { + t.Fatal("GenerateDetailed() error = nil, want notification error") + } + var notificationErr *NotificationError + if !errors.As(err, ¬ificationErr) { + t.Fatalf("GenerateDetailed() error = %T %v, want NotificationError", err, err) + } + if result == nil { + t.Fatal("GenerateDetailed() result = nil, want inspectable result on notification failure") + } + if result.Metadata.ReportID != report.Hourly || result.Metadata.NotificationPath != result.NotificationPath { + t.Fatalf("metadata = %#v notificationPath=%q, want hourly notification artifact link", result.Metadata, result.NotificationPath) + } + if result.NotificationPath == "" || result.ReportPath == "" || result.MetadataPath == "" { + t.Fatalf("result paths = report %q metadata %q notification %q, want inspectable artifact paths", result.ReportPath, result.MetadataPath, result.NotificationPath) + } + assertPathsExist(t, result.ReportPath, outputPath, result.MetadataPath, result.NotificationPath, result.GeneratedTextRawPath, result.GeneratedTextResultPath, result.GeneratedTextPath, result.RenderContextPath) + if result.Notification != nil { + t.Fatalf("Notification = %#v, want nil notification result when notifier returned only an error", result.Notification) + } + if len(notifier.requests) != 1 { + t.Fatalf("notification requests = %d, want one attempted notification", len(notifier.requests)) + } +} + func TestGenerateReportWritesReportAndPreflight(t *testing.T) { server := dailyBundleServer(t) cfg := dailyWorkspaceConfig(t, server) @@ -3496,6 +3587,66 @@ func resolveHourlyGeneratedTextFixture(t *testing.T, cfg config.Config) (report. return resolved, recordingFilesystemStore(t, cfg), &recordingNotifier{}, filepath.Join(t.TempDir(), "hourly-copy.md") } +func fakeScriptoriumBinary(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "scriptorium") + script := `#!/bin/sh +set -eu + +command_name="${1:-}" +shift || true +prompt="" +output="" +while [ "$#" -gt 0 ]; do + case "$1" in + --prompt) + shift + prompt="${1:-}" + ;; + --out) + shift + output="${1:-}" + ;; + esac + shift || true +done + +case "$command_name" in + render) + printf '{"prepared":true}\n' + ;; + run) + if [ -z "$output" ]; then + printf 'missing output path\n' >&2 + exit 2 + fi + case "$prompt" in + weather.hourly_generated_text) + cat > "$output" <<'EOF' +{"summary":"Storm chances increase through late morning.","forecast_discussion":"A front will keep the region unsettled.","precipitation_timing":"A cold front is moving into the region.","confidence":"Medium"} +EOF + ;; + *) + cat > "$output" <<'EOF' +# Generated Report + +Prepared report body. +EOF + ;; + esac + ;; + *) + printf 'unknown command: %s\n' "$command_name" >&2 + exit 2 + ;; +esac +` + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatalf("write fake scriptorium binary: %v", err) + } + return path +} + func validHourlyGeneratedTextJSON() string { return `{"summary":"Storm chances increase through late morning.","forecast_discussion":"A front will keep the region unsettled.","precipitation_timing":"A cold front is moving into the region.","confidence":"Medium"}` }