Complete comparison failure summaries

2026-08-02 13:28:16 +00:00
parent acb476a142
commit faf547e4a8
5 changed files with 217 additions and 20 deletions
--- a/internal/app/comparison.go
+++ b/internal/app/comparison.go
@@ -90,6 +90,11 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes
 		return nil, fmt.Errorf("build comparison identity: %w", err)
 	}
 	result := initialComparisonResult(req, resolved, comparisonID, now.UTC())
+	defer func() {
+		if result.FinishedAt.IsZero() {
+			finalizeComparisonResult(result, clock)
+		}
+	}()

 	outputName, err := resolved.OutputName()
 	if err != nil {
@@ -99,11 +104,11 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes
 	if err != nil {
 		return result, err
 	}
-	initialPlan, err := comparison.PlanDestination(req.WorkingDir, outputDirectory, req.Replace)
+	result.OutputDirectory = outputDirectory
+	_, err = comparison.PlanDestination(req.WorkingDir, outputDirectory, req.Replace)
 	if err != nil {
 		return result, fmt.Errorf("preflight comparison destination: %w", err)
 	}
-	result.OutputDirectory = initialPlan.Target

 	debugWriter, err := promptdebug.NewPromptDebugWriter(req.LLMDebugDir)
 	if err != nil {
@@ -129,7 +134,7 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes
 	executed := executeComparisonProfiles(ctx, comparisonExecutionRequest{
 		Prepared: prepared, Inspection: inspection, ComparisonID: comparisonID, DebugWriter: debugWriter, Executor: req.Executor,
 	})
-	result.FinishedAt = clock.Now().UTC()
+	finalizeComparisonResult(result, clock)
 	copyComparisonOutcomes(result, executed.Outcomes, false)
 	if executed.Canceled {
 		return result, fmt.Errorf("comparison execution: %w", ctx.Err())
@@ -160,6 +165,17 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes
 	return result, nil
 }

+func finalizeComparisonResult(result *ComparisonResult, clock timeutil.Clock) {
+	finishedAt := clock.Now().UTC()
+	if finishedAt.IsZero() {
+		finishedAt = time.Unix(0, 1).UTC()
+	}
+	if finishedAt.Before(result.StartedAt) {
+		finishedAt = result.StartedAt
+	}
+	result.FinishedAt = finishedAt
+}
+
 func initialComparisonResult(req ComparisonRequest, resolved report.Resolved, comparisonID string, startedAt time.Time) *ComparisonResult {
 	metadata := resolved.Metadata()
 	return &ComparisonResult{
--- a/internal/app/comparison_test.go
+++ b/internal/app/comparison_test.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"

 	"gitea.maximumdirect.net/eric/weatherreporter/internal/comparison"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
@@ -130,6 +131,78 @@ func TestCompareDetailedPreflightsBeforePromptOrCollection(t *testing.T) {
 	}
 }

+func TestCompareDetailedFinalizesUnpublishedFailures(t *testing.T) {
+	for _, test := range []struct {
+		name       string
+		prepare    func(t *testing.T, outputDirectory string)
+		debugDir   string
+		executor   *generationExecutor
+		collector  *generationCollector
+		wantPrompt bool
+	}{
+		{
+			name: "destination preflight",
+			prepare: func(t *testing.T, outputDirectory string) {
+				t.Helper()
+				if err := os.WriteFile(outputDirectory, []byte("not a directory"), 0o600); err != nil {
+					t.Fatal(err)
+				}
+			},
+			executor: &generationExecutor{},
+		},
+		{
+			name:      "debug initialization",
+			debugDir:  "relative-debug-directory",
+			executor:  &generationExecutor{},
+			collector: &generationCollector{},
+		},
+		{
+			name:       "prompt preflight",
+			executor:   &generationExecutor{inspectErr: promptexec.NewError(promptexec.PromptLoad, "unsafe prompt detail", errors.New("unsafe cause"))},
+			collector:  &generationCollector{},
+			wantPrompt: false,
+		},
+		{
+			name:       "collection",
+			executor:   &generationExecutor{},
+			collector:  &generationCollector{err: errors.New("collection failed")},
+			wantPrompt: true,
+		},
+		{
+			name:       "preparation",
+			executor:   &generationExecutor{},
+			collector:  &generationCollector{bundle: &weatherdata.Bundle{}},
+			wantPrompt: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			workingDirectory := t.TempDir()
+			outputDirectory := filepath.Join(workingDirectory, "comparison-output")
+			if test.prepare != nil {
+				test.prepare(t, outputDirectory)
+			}
+			collector := test.collector
+			if collector == nil {
+				bundle := generationBundle(t)
+				collector = &generationCollector{bundle: &bundle}
+			}
+			result, err := CompareDetailed(context.Background(), ComparisonRequest{
+				Config: comparisonConfig(), Report: ReportDaily, ProfileIDs: []string{"weather-light", "weather-deep"},
+				WorkingDir: workingDirectory, OutputDir: outputDirectory, LLMDebugDir: test.debugDir,
+				Date: generationTime("2026-05-29T12:00:00-05:00"), Clock: timeutil.FixedClock{Time: generationTime("2026-05-29T08:30:00-05:00")},
+				Collector: collector, Executor: test.executor,
+			})
+			if err == nil {
+				t.Fatal("CompareDetailed() error = nil")
+			}
+			assertUnpublishedComparisonResult(t, result, outputDirectory)
+			if (result.PromptID != "") != test.wantPrompt || (result.PromptHash != "") != test.wantPrompt {
+				t.Fatalf("prompt identity = %q/%q, want resolved=%t", result.PromptID, result.PromptHash, test.wantPrompt)
+			}
+		})
+	}
+}
+
 func TestCompareDetailedLeavesDestinationWhenCollectionOrPreparationFails(t *testing.T) {
 	collectionErr := errors.New("weather collection failed")
 	for _, test := range []struct {
@@ -233,3 +306,15 @@ func comparisonConfig() config.Config {
 	cfg.WeatherAPI.Timezone, cfg.Location.ID = "America/Chicago", "home"
 	return cfg
 }
+
+func assertUnpublishedComparisonResult(t *testing.T, result *ComparisonResult, outputDirectory string) {
+	t.Helper()
+	if result == nil || result.OutputDirectory != outputDirectory || !filepath.IsAbs(result.OutputDirectory) || result.FinishedAt.IsZero() || result.FinishedAt.Location() != time.UTC || result.FinishedAt.Before(result.StartedAt) || result.ManifestPath != "" || result.DataPackagePath != "" {
+		t.Fatalf("unpublished comparison result = %#v", result)
+	}
+	for _, profile := range result.Results {
+		if profile.ReportPath != "" {
+			t.Fatalf("unpublished profile result = %#v", profile)
+		}
+	}
+}
--- a/internal/cli/comparison_test.go
+++ b/internal/cli/comparison_test.go
@@ -237,7 +237,7 @@ func TestCompareCommandReportsCommittedBundleWhenCleanupFails(t *testing.T) {
 	if err := json.Unmarshal(stdout.Bytes(), &summary); err != nil {
 		t.Fatalf("decode summary: %v\n%s", err, stdout.String())
 	}
-	if summary.Status != summaryStatusFailed || summary.Error == nil || summary.Error.Message != "comparison did not complete" || summary.ManifestPath != result.ManifestPath || summary.DataPackagePath != result.DataPackagePath || summary.Results[0].ReportPath == "" {
+	if summary.Status != summaryStatusFailed || summary.Error == nil || summary.Error.Category != "publication_cleanup" || summary.Error.Message != "comparison published but cleanup did not complete" || summary.ManifestPath != result.ManifestPath || summary.DataPackagePath != result.DataPackagePath || summary.Results[0].ReportPath == "" {
 		t.Fatalf("summary = %#v", summary)
 	}
 	for _, unsafe := range []string{cleanupCause.Error(), backupPath} {
--- a/internal/cli/result.go
+++ b/internal/cli/result.go
@@ -10,6 +10,7 @@ import (

 	"gitea.maximumdirect.net/eric/weatherreporter/internal/app"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/comparison"
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/promptexec"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/weatherdata"
@@ -216,29 +217,50 @@ func comparisonSummaryStatus(result *app.ComparisonResult, err error) string {
 }

 func safeComparisonSummaryError(err error) *comparison.SafeError {
-	message := "comparison did not complete"
-	if aggregate, ok := comparisonAggregateErrorMessage(err.Error()); ok {
-		message = aggregate
-	} else if errors.Is(err, context.DeadlineExceeded) {
-		message = "comparison deadline exceeded"
-	} else if errors.Is(err, context.Canceled) {
-		message = "comparison canceled"
+	if aggregate, ok := comparisonAggregateErrorMessage(err); ok {
+		safe := comparison.NewSafeError("application", aggregate)
+		return &safe
 	}
-	safe := comparison.NewSafeError("application", message)
+	var cleanupErr *comparison.PublicationCleanupError
+	if errors.As(err, &cleanupErr) {
+		safe := comparison.NewSafeError("publication_cleanup", "comparison published but cleanup did not complete")
+		return &safe
+	}
+	var destinationErr *comparison.DestinationError
+	if errors.As(err, &destinationErr) {
+		safe := comparison.NewSafeError("destination_"+string(destinationErr.Kind), "comparison destination preflight failed")
+		return &safe
+	}
+	if category := promptexec.CategoryOf(err); category != "" {
+		safe := comparison.NewSafeError(string(category), "comparison prompt operation failed")
+		return &safe
+	}
+	if errors.Is(err, context.DeadlineExceeded) {
+		safe := comparison.NewSafeError("deadline_exceeded", "comparison deadline exceeded")
+		return &safe
+	}
+	if errors.Is(err, context.Canceled) {
+		safe := comparison.NewSafeError("canceled", "comparison canceled")
+		return &safe
+	}
+	safe := comparison.NewSafeError("application", "comparison did not complete")
 	return &safe
 }

-func comparisonAggregateErrorMessage(value string) (string, bool) {
+func comparisonAggregateErrorMessage(err error) (string, bool) {
 	const prefix = "comparison completed with "
 	const suffix = " failed profiles"
-	if !strings.HasPrefix(value, prefix) || !strings.HasSuffix(value, suffix) {
-		return "", false
+	for candidate := err; candidate != nil; candidate = errors.Unwrap(candidate) {
+		value := candidate.Error()
+		if !strings.HasPrefix(value, prefix) || !strings.HasSuffix(value, suffix) {
+			continue
+		}
+		count, parseErr := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(value, prefix), suffix))
+		if parseErr == nil && count > 0 {
+			return fmt.Sprintf("comparison completed with %d failed profiles", count), true
+		}
 	}
-	count, err := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(value, prefix), suffix))
-	if err != nil || count < 1 {
-		return "", false
-	}
-	return fmt.Sprintf("comparison completed with %d failed profiles", count), true
+	return "", false
 }

 func batchSummaryStatus(result *app.BatchResult) string {
--- a/internal/cli/result_test.go
+++ b/internal/cli/result_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -135,3 +136,76 @@ func TestComparisonSummaryClassifiesCompleteAndAllFailedResults(t *testing.T) {
 		t.Fatalf("all-failed summary = %#v", summary)
 	}
 }
+
+func TestSafeComparisonSummaryErrorClassifiesWrappedFailures(t *testing.T) {
+	unsafeDetail := "unsafe filesystem and provider detail"
+	unsafeCause := errors.New(unsafeDetail)
+	for _, test := range []struct {
+		name     string
+		err      error
+		category string
+		message  string
+	}{
+		{
+			name:     "aggregate profile failure",
+			err:      fmt.Errorf("outer wrapper: %w", errors.New("comparison completed with 2 failed profiles")),
+			category: "application",
+			message:  "comparison completed with 2 failed profiles",
+		},
+		{
+			name:     "canceled",
+			err:      fmt.Errorf("outer wrapper: %w", context.Canceled),
+			category: "canceled",
+			message:  "comparison canceled",
+		},
+		{
+			name:     "deadline exceeded",
+			err:      fmt.Errorf("outer wrapper: %w", context.DeadlineExceeded),
+			category: "deadline_exceeded",
+			message:  "comparison deadline exceeded",
+		},
+		{
+			name:     "prompt operation",
+			err:      fmt.Errorf("outer wrapper: %w", promptexec.NewError(promptexec.Generation, "unsafe prompt detail", fmt.Errorf("%w: %s", context.Canceled, unsafeDetail))),
+			category: string(promptexec.Generation),
+			message:  "comparison prompt operation failed",
+		},
+		{
+			name: "destination preflight",
+			err: fmt.Errorf("outer wrapper: %w", &comparison.DestinationError{
+				Kind: comparison.DestinationNotEmpty, Target: "/tmp/" + unsafeDetail, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
+			}),
+			category: "destination_not_empty",
+			message:  "comparison destination preflight failed",
+		},
+		{
+			name: "publication cleanup",
+			err: fmt.Errorf("outer wrapper: %w", &comparison.PublicationCleanupError{
+				RetainedBackupPath: "/tmp/" + unsafeDetail, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
+			}),
+			category: "publication_cleanup",
+			message:  "comparison published but cleanup did not complete",
+		},
+		{
+			name:     "unknown",
+			err:      fmt.Errorf("outer wrapper: %w", unsafeCause),
+			category: "application",
+			message:  "comparison did not complete",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			result := &app.ComparisonResult{ComparisonID: "comparison_test", ReportID: report.Daily}
+			summary := newComparisonSummary(result, test.err)
+			if summary.Error == nil || summary.Error.Category != test.category || summary.Error.Message != test.message {
+				t.Fatalf("summary error = %#v, want %q/%q", summary.Error, test.category, test.message)
+			}
+			data, err := json.Marshal(summary)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if strings.Contains(string(data), unsafeDetail) {
+				t.Fatalf("summary includes unsafe detail: %s", data)
+			}
+		})
+	}
+}