From 42f0e16b02e2fd8f5620fa2175a6bff3a4a87ba2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 17 Jun 2026 14:39:53 -0500 Subject: [PATCH] Fix to ensure unique document IDs when batch reports are generated --- docs/operations.md | 8 ++- internal/app/app_test.go | 97 ++++++++++++++++++++++++++++++- internal/report/daily_report.go | 12 ++++ internal/report/definition.go | 16 ++++- internal/report/period_test.go | 41 +++++++++++-- internal/state/filesystem_test.go | 58 ++++++++++++++---- 6 files changed, 213 insertions(+), 19 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index cc90aae..4d05136 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -203,10 +203,14 @@ period do not overwrite each other. ## RunID And Metadata -RunIDs are based on generation time plus report ID: +RunIDs are based on generation time plus report ID. Reports that can be +generated more than once in a single command may append a report-specific +disambiguator. Daily appends the local valid date so multiple dynamic Daily +reports in one batch have distinct managed artifacts and notification +idempotency keys: ```text -20260529T100000.123456789Z_daily +20260529T100000.123456789Z_daily_2026-05-31 20260529T100000.123456789Z_today ``` diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 21f0863..1a5557e 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -2409,6 +2409,97 @@ func TestRunBatchUsesOutputDirectory(t *testing.T) { } } +func TestRunBatchDynamicDailyReportsHaveDistinctIdentity(t *testing.T) { + server := dailyBundleServer(t) + cfg := dailyNotificationConfig(t, server) + collection := collectionWithFutureDailyForTest(t, cfg, "2026-05-31", "2026-06-01") + cfg.WeatherAPI.BaseURL = "" + collector := &recordingCollector{result: &collection} + notifier := &recordingNotifier{} + outputDir := filepath.Join(t.TempDir(), "reports") + + result, err := RunBatchDetailed(context.Background(), BatchRequest{ + Config: cfg, + Batch: BatchEvening, + Now: mustParse("2026-05-29T18:00:00-05:00"), + OutputDir: outputDir, + Collector: collector, + Renderer: &selectiveRenderer{runBody: "# Batch Report\n"}, + Notifier: notifier, + }) + if err != nil { + t.Fatalf("RunBatchDetailed() error = %v", err) + } + if result.Failed != 0 || len(result.Reports) != 3 { + t.Fatalf("summary = %#v, want three successful reports", result) + } + if len(notifier.requests) != 3 { + t.Fatalf("notification requests = %d, want one per report", len(notifier.requests)) + } + + dailyByDate := map[string]BatchReportResult{} + runIDs := map[string]struct{}{} + reportPaths := map[string]struct{}{} + metadataPaths := map[string]struct{}{} + dataPackagePaths := map[string]struct{}{} + for _, item := range result.Reports { + if _, ok := runIDs[item.RunID]; ok { + t.Fatalf("duplicate RunID in batch result: %q", item.RunID) + } + runIDs[item.RunID] = struct{}{} + if _, ok := reportPaths[item.ReportPath]; ok { + t.Fatalf("duplicate ReportPath in batch result: %q", item.ReportPath) + } + reportPaths[item.ReportPath] = struct{}{} + if _, ok := metadataPaths[item.MetadataPath]; ok { + t.Fatalf("duplicate MetadataPath in batch result: %q", item.MetadataPath) + } + metadataPaths[item.MetadataPath] = struct{}{} + if _, ok := dataPackagePaths[item.DataPackagePath]; ok { + t.Fatalf("duplicate DataPackagePath in batch result: %q", item.DataPackagePath) + } + dataPackagePaths[item.DataPackagePath] = struct{}{} + if item.ReportID == report.Daily { + if !strings.HasPrefix(item.RunID, "20260529T230000.000000000Z_daily_") { + t.Fatalf("Daily RunID = %q, want dated daily run id", item.RunID) + } + date := strings.TrimPrefix(filepath.Base(item.OutputPath), "daily-") + date = strings.TrimSuffix(date, ".md") + dailyByDate[date] = item + } + } + + for _, date := range []string{"2026-05-31", "2026-06-01"} { + item, ok := dailyByDate[date] + if !ok { + t.Fatalf("daily outputs = %#v, want Daily output for %s", dailyByDate, date) + } + if item.RunID != "20260529T230000.000000000Z_daily_"+date { + t.Fatalf("Daily %s RunID = %q, want date disambiguator", date, item.RunID) + } + if item.OutputPath != filepath.Join(outputDir, "daily-"+date+".md") { + t.Fatalf("Daily %s OutputPath = %q, want date-qualified copy", date, item.OutputPath) + } + } + + dailyKeys := map[string]struct{}{} + for _, req := range notifier.requests { + if req.ReportID != report.Daily { + continue + } + if req.IdempotencyKey != req.BundleID+"."+req.RunID { + t.Fatalf("Daily idempotency key = %q, want bundle id plus run id", req.IdempotencyKey) + } + if _, ok := dailyKeys[req.IdempotencyKey]; ok { + t.Fatalf("duplicate Daily idempotency key: %q", req.IdempotencyKey) + } + dailyKeys[req.IdempotencyKey] = struct{}{} + } + if len(dailyKeys) != 2 { + t.Fatalf("Daily notification keys = %#v, want two distinct keys", dailyKeys) + } +} + func TestRunBatchMorningUsesTodayOutputName(t *testing.T) { server := dailyBundleServer(t) cfg := dailyWorkspaceConfig(t, server) @@ -2514,11 +2605,13 @@ func collectionForTest(t *testing.T, cfg config.Config) collect.Result { return collect.Result{Bundle: bundle} } -func collectionWithFutureDailyForTest(t *testing.T, cfg config.Config, date string) collect.Result { +func collectionWithFutureDailyForTest(t *testing.T, cfg config.Config, dates ...string) collect.Result { t.Helper() collection := collectionForTest(t, cfg) location := mustLoadTestLocation(t, cfg.WeatherAPI.Timezone) - collection.Bundle.Hourly.Periods = append(collection.Bundle.Hourly.Periods, fullDayPeriods(t, date, location)...) + for _, date := range dates { + collection.Bundle.Hourly.Periods = append(collection.Bundle.Hourly.Periods, fullDayPeriods(t, date, location)...) + } return collection } diff --git a/internal/report/daily_report.go b/internal/report/daily_report.go index de57f9f..c2bdd51 100644 --- a/internal/report/daily_report.go +++ b/internal/report/daily_report.go @@ -22,6 +22,7 @@ func dailyDefinition() Definition { CompatiblePriorIDs: []ID{Daily}, Modules: dailyModules(), resolve: resolveDaily, + runIDDisambiguator: validStartDateRunIDDisambiguator, } } @@ -50,3 +51,14 @@ func resolveDaily(req ResolveRequest) (timeutil.Period, error) { } return timeutil.CivilDay(req.Date, req.Location), nil } + +func validStartDateRunIDDisambiguator(resolved Resolved) string { + if resolved.ValidPeriod.Start.IsZero() { + return "" + } + location, err := timeutil.LoadLocation(resolved.Timezone) + if err != nil { + return resolved.ValidPeriod.Start.Format(timeutil.DateLayout) + } + return resolved.ValidPeriod.Start.In(location).Format(timeutil.DateLayout) +} diff --git a/internal/report/definition.go b/internal/report/definition.go index a912d74..b567ea2 100644 --- a/internal/report/definition.go +++ b/internal/report/definition.go @@ -3,6 +3,7 @@ package report import ( "fmt" + "strings" "time" "gitea.maximumdirect.net/eric/weatherreporter/internal/module" @@ -60,6 +61,7 @@ type Definition struct { Morning bool Evening bool resolve func(ResolveRequest) (timeutil.Period, error) + runIDDisambiguator func(Resolved) string } func (d Definition) ResolvePeriod(req ResolveRequest) (timeutil.Period, error) { @@ -112,7 +114,7 @@ type Metadata struct { func (r Resolved) Metadata() Metadata { return Metadata{ - RunID: r.GeneratedAt.UTC().Format("20060102T150405.000000000Z") + "_" + string(r.Definition.ID), + RunID: r.runID(), ReportID: r.Definition.ID, PromptID: r.Definition.PromptID, GeneratedAt: r.GeneratedAt, @@ -120,3 +122,15 @@ func (r Resolved) Metadata() Metadata { ValidPeriod: r.ValidPeriod, } } + +func (r Resolved) runID() string { + runID := r.GeneratedAt.UTC().Format("20060102T150405.000000000Z") + "_" + string(r.Definition.ID) + if r.Definition.runIDDisambiguator == nil { + return runID + } + disambiguator := strings.TrimSpace(r.Definition.runIDDisambiguator(r)) + if disambiguator == "" { + return runID + } + return runID + "_" + disambiguator +} diff --git a/internal/report/period_test.go b/internal/report/period_test.go index 6e9a982..31a1af3 100644 --- a/internal/report/period_test.go +++ b/internal/report/period_test.go @@ -713,8 +713,41 @@ func TestResolvedMetadata(t *testing.T) { if metadata.PromptID != "weather.daily_generated_text" { t.Fatalf("PromptID = %q, want weather.daily_generated_text", metadata.PromptID) } - if !strings.Contains(metadata.RunID, "daily") { - t.Fatalf("RunID = %q, want report id", metadata.RunID) + if metadata.RunID != "20260529T100000.000000000Z_daily_2026-05-29" { + t.Fatalf("RunID = %q, want dated Daily report id", metadata.RunID) + } +} + +func TestDailyRunIDIncludesValidDateDisambiguator(t *testing.T) { + location := mustLoadLocation(t) + now := mustParse("2026-05-29T05:00:00-05:00") + first, err := Resolve(Daily, ResolveRequest{ + Now: now, + Location: location, + Date: mustParse("2026-05-31T12:00:00-05:00"), + }) + if err != nil { + t.Fatalf("Resolve(first) error = %v", err) + } + second, err := Resolve(Daily, ResolveRequest{ + Now: now, + Location: location, + Date: mustParse("2026-06-01T12:00:00-05:00"), + }) + if err != nil { + t.Fatalf("Resolve(second) error = %v", err) + } + + firstRunID := first.Metadata().RunID + secondRunID := second.Metadata().RunID + if firstRunID == secondRunID { + t.Fatalf("Daily RunIDs both = %q, want distinct valid-date suffixes", firstRunID) + } + if firstRunID != "20260529T100000.000000000Z_daily_2026-05-31" { + t.Fatalf("first RunID = %q, want valid-date suffix", firstRunID) + } + if secondRunID != "20260529T100000.000000000Z_daily_2026-06-01" { + t.Fatalf("second RunID = %q, want valid-date suffix", secondRunID) } } @@ -731,8 +764,8 @@ func TestHourlyMetadataRunIDIncludesReportID(t *testing.T) { if metadata.PromptID != "weather.hourly_generated_text" { t.Fatalf("PromptID = %q, want weather.hourly_generated_text", metadata.PromptID) } - if !strings.Contains(metadata.RunID, "hourly") { - t.Fatalf("RunID = %q, want report id", metadata.RunID) + if metadata.RunID != "20260529T101500.000000000Z_hourly" { + t.Fatalf("RunID = %q, want unchanged report id shape", metadata.RunID) } } diff --git a/internal/state/filesystem_test.go b/internal/state/filesystem_test.go index 1c9062f..a963541 100644 --- a/internal/state/filesystem_test.go +++ b/internal/state/filesystem_test.go @@ -27,12 +27,12 @@ func TestPathsUseRunIDAndWorkspace(t *testing.T) { } for _, want := range []string{ - filepath.Join("snapshots", "daily", "2026-05-29", "20260529T100000.000000000Z_daily.modules.json"), - filepath.Join("snapshots", "daily", "2026-05-29", "20260529T100000.000000000Z_daily.metadata.json"), - filepath.Join("data-packages", "daily", "2026-05-29", "20260529T100000.000000000Z_daily.data_package.yaml"), - filepath.Join("preflight", "daily", "2026-05-29", "20260529T100000.000000000Z_daily.render.json"), - filepath.Join("notifications", "daily", "2026-05-29", "20260529T100000.000000000Z_daily.distributor.json"), - filepath.Join("reports", "daily", "20260529T100000.000000000Z_daily.md"), + filepath.Join("snapshots", "daily", "2026-05-29", "20260529T100000.000000000Z_daily_2026-05-29.modules.json"), + filepath.Join("snapshots", "daily", "2026-05-29", "20260529T100000.000000000Z_daily_2026-05-29.metadata.json"), + filepath.Join("data-packages", "daily", "2026-05-29", "20260529T100000.000000000Z_daily_2026-05-29.data_package.yaml"), + filepath.Join("preflight", "daily", "2026-05-29", "20260529T100000.000000000Z_daily_2026-05-29.render.json"), + filepath.Join("notifications", "daily", "2026-05-29", "20260529T100000.000000000Z_daily_2026-05-29.distributor.json"), + filepath.Join("reports", "daily", "20260529T100000.000000000Z_daily_2026-05-29.md"), } { if !strings.Contains(pathsString(paths), want) { t.Fatalf("paths = %#v, want component %q", paths, want) @@ -40,6 +40,35 @@ func TestPathsUseRunIDAndWorkspace(t *testing.T) { } } +func TestDailyPathsUseRunIDValidDateDisambiguator(t *testing.T) { + store := newTestStore(t) + first := resolveDailyForDateAt(t, "2026-05-29T05:00:00-05:00", "2026-05-31T12:00:00-05:00") + second := resolveDailyForDateAt(t, "2026-05-29T05:00:00-05:00", "2026-06-01T12:00:00-05:00") + + firstPaths, err := store.Paths(first) + if err != nil { + t.Fatalf("Paths(first) error = %v", err) + } + secondPaths, err := store.Paths(second) + if err != nil { + t.Fatalf("Paths(second) error = %v", err) + } + + if first.Metadata().RunID == second.Metadata().RunID { + t.Fatalf("RunIDs both = %q, want distinct daily run ids", first.Metadata().RunID) + } + for name, values := range map[string][2]string{ + "RenderedReport": {firstPaths.RenderedReport, secondPaths.RenderedReport}, + "Metadata": {firstPaths.Metadata, secondPaths.Metadata}, + "DataPackage": {firstPaths.DataPackage, secondPaths.DataPackage}, + "Notification": {firstPaths.Notification, secondPaths.Notification}, + } { + if values[0] == values[1] { + t.Fatalf("%s paths both = %q, want distinct daily artifact paths", name, values[0]) + } + } +} + func TestGeneratedTextArtifactPathsUseSnapshotTree(t *testing.T) { store := newTestStore(t) tests := []struct { @@ -54,7 +83,7 @@ func TestGeneratedTextArtifactPathsUseSnapshotTree(t *testing.T) { resolved: resolveDailyAt(t, "2026-05-29T05:00:00-05:00"), group: "daily", validDate: "2026-05-29", - runID: "20260529T100000.000000000Z_daily", + runID: "20260529T100000.000000000Z_daily_2026-05-29", }, { name: "hourly", @@ -567,19 +596,28 @@ func newTestStore(t *testing.T) *FilesystemStore { } func resolveDailyAt(t *testing.T, value string) report.Resolved { + t.Helper() + return resolveDailyForDateAt(t, value, value) +} + +func resolveDailyForDateAt(t *testing.T, nowValue string, dateValue string) report.Resolved { t.Helper() location, err := timeutil.LoadLocation("America/Chicago") if err != nil { t.Fatalf("LoadLocation() error = %v", err) } - now, err := time.Parse(time.RFC3339, value) + now, err := time.Parse(time.RFC3339, nowValue) if err != nil { - t.Fatalf("parse time: %v", err) + t.Fatalf("parse now time: %v", err) + } + date, err := time.Parse(time.RFC3339, dateValue) + if err != nil { + t.Fatalf("parse date time: %v", err) } resolved, err := report.DefaultRegistry().Resolve(report.Daily, report.ResolveRequest{ Now: now, Location: location, - Date: now, + Date: date, }) if err != nil { t.Fatalf("Resolve() error = %v", err)