From 40b42f4bf3e4d2e4234abb9faedda50549cde1b0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 15 Jun 2026 12:33:55 +0000 Subject: [PATCH] Centralize report name resolution --- docs/config.md | 6 +- docs/internal/report-registry.md | 31 ++++++--- internal/app/app.go | 50 +++------------ internal/cli/root.go | 38 ++--------- internal/cli/root_test.go | 31 +++++++++ internal/config/config_test.go | 32 ++++++++- internal/config/reports.go | 27 +------- internal/report/names.go | 86 +++++++++++++++++++++++++ internal/report/period_test.go | 107 +++++++++++++++++++++++++++++++ 9 files changed, 297 insertions(+), 111 deletions(-) create mode 100644 internal/report/names.go diff --git a/docs/config.md b/docs/config.md index d0c860a..d4d1597 100644 --- a/docs/config.md +++ b/docs/config.md @@ -182,8 +182,10 @@ snapshot exists and a threshold is crossed. report definitions. Omit a report entry to use its default module order. Supported report keys are `daily`, `tomorrow`, `hourly`, `three_day`, -`weekend`, and `storm`. Canonical report IDs such as `daily_today` are also -accepted. +`weekend`, and `storm`. Canonical report IDs and accepted aliases are also +valid, including `daily_today`, `three_day_outlook`, `weekend_outlook`, and +`storm_report`. Hyphens and underscores are treated equivalently in report +keys. Each report entry supports: diff --git a/docs/internal/report-registry.md b/docs/internal/report-registry.md index 0905df6..a9a32f5 100644 --- a/docs/internal/report-registry.md +++ b/docs/internal/report-registry.md @@ -6,9 +6,10 @@ membership, output naming, artifact grouping, and comparison declarations in ## Purpose -`internal/report` is the canonical source for report definitions. App, state, -module building, and CLI wiring consume resolved definitions instead of owning -report identity policy themselves. +`internal/report` is the canonical source for report definitions, public +command names, config-key aliases, and batch command names. App, config, state, +module building, and CLI wiring consume report-owned helpers and resolved +definitions instead of owning report identity policy themselves. ## Definition Fields @@ -26,6 +27,12 @@ Each report definition declares: - morning or evening batch membership - default ordered module composition +Report-owned helpers map public command names and config keys to report IDs. +The generate command names are `daily`, `tomorrow`, `hourly`, `three-day`, +`weekend`, and `storm`. Config keys also accept underscore and legacy +descriptive aliases such as `daily_today`, `three_day_outlook`, +`weekend_outlook`, and `storm_report`. + Markdown report definitions use the `scriptorium_markdown` generation mode. Their template and structured-text schema identifiers are empty. Tomorrow Report and Hourly Report declare `generated_text_template`; the app uses their @@ -65,19 +72,21 @@ must be after start time. ## Boundaries -`internal/report` defines report metadata and time coverage. It does not fetch -weather data, build module values, compare snapshot contents, write state, -parse CLI flags, or invoke Scriptorium. +`internal/report` defines report metadata, public report names, batch command +names, and time coverage. It does not fetch weather data, build module values, +compare snapshot contents, write state, parse CLI flags, or invoke Scriptorium. -The CLI owns public command names. The app maps those command names to report -IDs, then uses the registry for report policy. +The CLI parses flags and command structure, then uses report-owned helpers for +report and batch command names. Config loading uses report-owned helpers for +report override keys. ## Config Fields Used The app supplies `weather_api.timezone` as a loaded `time.Location`. Batch output path copying uses batch output names from report definitions. Report -module overrides can use short keys such as `tomorrow` and `hourly`, or -canonical report IDs such as `daily_today`. +module overrides can use short keys such as `tomorrow` and `hourly`, canonical +report IDs such as `daily_today`, or accepted aliases such as +`three_day_outlook`. ## Batch Membership @@ -111,6 +120,8 @@ Inspect: ## Invariants - Report selection goes through the registry. +- Public command names, config-key aliases, and batch command names are owned + by `internal/report`. - Direct Markdown reports have empty template and generated-text schema IDs. - Generated-text-template reports declare prompt, template, and schema IDs in their report definition. diff --git a/internal/app/app.go b/internal/app/app.go index 7b11d53..c179ff2 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -29,19 +29,19 @@ import ( type ReportKind string const ( - ReportDaily ReportKind = "daily" - ReportTomorrow ReportKind = "tomorrow" - ReportHourly ReportKind = "hourly" - ReportThreeDay ReportKind = "three-day" - ReportWeekend ReportKind = "weekend" - ReportStorm ReportKind = "storm" + ReportDaily ReportKind = ReportKind(report.CommandNameDaily) + ReportTomorrow ReportKind = ReportKind(report.CommandNameTomorrow) + ReportHourly ReportKind = ReportKind(report.CommandNameHourly) + ReportThreeDay ReportKind = ReportKind(report.CommandNameThreeDay) + ReportWeekend ReportKind = ReportKind(report.CommandNameWeekend) + ReportStorm ReportKind = ReportKind(report.CommandNameStorm) ) type BatchKind string const ( - BatchMorning BatchKind = "morning" - BatchEvening BatchKind = "evening" + BatchMorning BatchKind = BatchKind(report.BatchNameMorning) + BatchEvening BatchKind = BatchKind(report.BatchNameEvening) ) type GenerateRequest struct { @@ -344,7 +344,7 @@ func ResolveGenerate(req GenerateRequest, now time.Time) (report.Resolved, error if err != nil { return report.Resolved{}, err } - id, err := reportIDForCommand(req.Report) + id, err := report.IDForCommandName(string(req.Report)) if err != nil { return report.Resolved{}, err } @@ -366,7 +366,7 @@ func ResolveBatch(req BatchRequest, now time.Time) ([]report.Resolved, error) { if err != nil { return nil, err } - batch, err := reportBatchForCommand(req.Batch) + batch, err := report.BatchForCommandName(string(req.Batch)) if err != nil { return nil, err } @@ -388,36 +388,6 @@ func reportRegistry(cfg config.Config) (report.Registry, error) { return registry, nil } -func reportIDForCommand(kind ReportKind) (report.ID, error) { - switch kind { - case ReportDaily: - return report.DailyToday, nil - case ReportTomorrow: - return report.Tomorrow, nil - case ReportHourly: - return report.Hourly, nil - case ReportThreeDay: - return report.ThreeDay, nil - case ReportWeekend: - return report.Weekend, nil - case ReportStorm: - return report.Storm, nil - default: - return "", fmt.Errorf("unknown report command %q", kind) - } -} - -func reportBatchForCommand(kind BatchKind) (report.Batch, error) { - switch kind { - case BatchMorning: - return report.Morning, nil - case BatchEvening: - return report.Evening, nil - default: - return "", fmt.Errorf("unknown batch command %q", kind) - } -} - func FetchBundle(ctx context.Context, req FetchBundleRequest) (*weatherdata.Bundle, error) { client, err := weatherapi.New(req.Config) if err != nil { diff --git a/internal/cli/root.go b/internal/cli/root.go index 7b954ca..e0bbcc4 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -186,10 +186,10 @@ func (r Runner) resolveGenerate(args []string) (app.GenerateRequest, error) { if len(args) == 0 { return app.GenerateRequest{}, fmt.Errorf("generate requires a report name") } - reportKind, ok := reportKind(args[0]) - if !ok { + if _, err := report.IDForCommandName(args[0]); err != nil { return app.GenerateRequest{}, fmt.Errorf("unknown generate report %q", args[0]) } + reportKind := app.ReportKind(args[0]) opts, err := parseGenerateFlags(reportKind, args[1:]) if err != nil { @@ -250,10 +250,10 @@ func (r Runner) resolveRun(args []string) (app.BatchRequest, error) { if len(args) == 0 { return app.BatchRequest{}, fmt.Errorf("run requires a batch name") } - batch, ok := batchKind(args[0]) - if !ok { + if _, err := report.BatchForCommandName(args[0]); err != nil { return app.BatchRequest{}, fmt.Errorf("unknown run batch %q", args[0]) } + batch := app.BatchKind(args[0]) opts, err := parseRunFlags(args[1:]) if err != nil { return app.BatchRequest{}, err @@ -380,33 +380,3 @@ func addCommonFlags(fs *flag.FlagSet, opts *commonOptions, includeOutput bool) { fs.StringVar(&opts.Output, "out", "", "extra Markdown report copy path") } } - -func reportKind(value string) (app.ReportKind, bool) { - switch value { - case string(app.ReportDaily): - return app.ReportDaily, true - case string(app.ReportTomorrow): - return app.ReportTomorrow, true - case string(app.ReportHourly): - return app.ReportHourly, true - case string(app.ReportThreeDay): - return app.ReportThreeDay, true - case string(app.ReportWeekend): - return app.ReportWeekend, true - case string(app.ReportStorm): - return app.ReportStorm, true - default: - return "", false - } -} - -func batchKind(value string) (app.BatchKind, bool) { - switch value { - case string(app.BatchMorning): - return app.BatchMorning, true - case string(app.BatchEvening): - return app.BatchEvening, true - default: - return "", false - } -} diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 4f9e7e6..d31c985 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -13,6 +13,7 @@ import ( "time" "gitea.maximumdirect.net/eric/weatherreporter/internal/app" + "gitea.maximumdirect.net/eric/weatherreporter/internal/report" "gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil" ) @@ -892,6 +893,36 @@ func TestResolveGenerateCommands(t *testing.T) { } } +func TestResolveGenerateSupportsEveryReportCommandName(t *testing.T) { + runner := Runner{Clock: fixedClock()} + for _, name := range report.CommandNames() { + t.Run(name, func(t *testing.T) { + args := []string{name} + if name == report.CommandNameDaily { + args = append(args, "--date", "2026-05-29") + } + if name == report.CommandNameStorm { + args = append(args, "--start", "2026-05-29T18:00", "--end", "2026-05-30T06:00") + } + req, err := runner.resolveGenerate(args) + if err != nil { + t.Fatalf("resolveGenerate() error = %v", err) + } + want, err := report.IDForCommandName(name) + if err != nil { + t.Fatalf("IDForCommandName() error = %v", err) + } + resolved, err := app.ResolveGenerate(req, req.Now) + if err != nil { + t.Fatalf("ResolveGenerate() error = %v", err) + } + if resolved.Definition.ID != want { + t.Fatalf("resolved ID = %q, want %q", resolved.Definition.ID, want) + } + }) + } +} + func TestResolveGenerateHourlyAppliesSharedFlags(t *testing.T) { runner := Runner{Clock: fixedClock()} configPath := filepath.Join(t.TempDir(), "config.yml") diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d3a6404..850ba89 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -205,6 +205,36 @@ reports: } } +func TestLoadReportModuleOverrideAliases(t *testing.T) { + path := writeConfig(t, ` +reports: + three-day-outlook: + deterministic_modules: + - metadata + weekend_outlook: + deterministic_modules: + - metadata + storm_report: + deterministic_modules: + - metadata +`) + + cfg, err := LoadFile(path) + if err != nil { + t.Fatalf("LoadFile() error = %v", err) + } + overrides := cfg.ReportModuleOverrides() + if len(overrides[report.ThreeDay]) != 1 || overrides[report.ThreeDay][0].ID != module.Metadata { + t.Fatalf("three-day alias override = %#v, want metadata override", overrides[report.ThreeDay]) + } + if len(overrides[report.Weekend]) != 1 || overrides[report.Weekend][0].ID != module.Metadata { + t.Fatalf("weekend alias override = %#v, want metadata override", overrides[report.Weekend]) + } + if len(overrides[report.Storm]) != 1 || overrides[report.Storm][0].ID != module.Metadata { + t.Fatalf("storm alias override = %#v, want metadata override", overrides[report.Storm]) + } +} + func TestReportModuleOverrideValidation(t *testing.T) { tests := []struct { name string @@ -270,7 +300,7 @@ reports: deterministic_modules: - metadata `, - wantErr: "reports.daily_tomorrow is not a known report", + wantErr: "reports.daily_tomorrow", }, { name: "InvalidOptions", diff --git a/internal/config/reports.go b/internal/config/reports.go index cf019a3..a901170 100644 --- a/internal/config/reports.go +++ b/internal/config/reports.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "reflect" - "strings" "gitea.maximumdirect.net/eric/weatherreporter/internal/briefing" "gitea.maximumdirect.net/eric/weatherreporter/internal/module" @@ -18,7 +17,7 @@ func (cfg Config) ReportModuleOverrides() map[report.ID][]module.ConfigItem { if !reportCfg.deterministicModulesSet { continue } - id, err := reportIDForConfigKey(key) + id, err := report.IDForConfigKey(key) if err != nil { continue } @@ -42,9 +41,9 @@ func normalizeReportModules(cfg *Config) error { reportRegistry := report.DefaultRegistry() seenReports := map[report.ID]string{} for key, reportCfg := range cfg.Reports { - reportID, err := reportIDForConfigKey(key) + reportID, err := report.IDForConfigKey(key) if err != nil { - return err + return fmt.Errorf("reports.%s: %w", key, err) } if previous, ok := seenReports[reportID]; ok { return fmt.Errorf("reports.%s duplicates report override %q", key, previous) @@ -105,23 +104,3 @@ func decodeKnownOptions(raw any, optionType reflect.Type) (any, error) { } return target.Elem().Interface(), nil } - -func reportIDForConfigKey(key string) (report.ID, error) { - normalized := strings.ReplaceAll(strings.TrimSpace(strings.ToLower(key)), "-", "_") - switch normalized { - case "daily", "daily_today": - return report.DailyToday, nil - case "tomorrow": - return report.Tomorrow, nil - case "hourly": - return report.Hourly, nil - case "three_day", "three_day_outlook": - return report.ThreeDay, nil - case "weekend", "weekend_outlook": - return report.Weekend, nil - case "storm", "storm_report": - return report.Storm, nil - default: - return "", fmt.Errorf("reports.%s is not a known report", key) - } -} diff --git a/internal/report/names.go b/internal/report/names.go new file mode 100644 index 0000000..9c5af95 --- /dev/null +++ b/internal/report/names.go @@ -0,0 +1,86 @@ +package report + +import ( + "fmt" + "strings" +) + +const ( + CommandNameDaily = "daily" + CommandNameTomorrow = "tomorrow" + CommandNameHourly = "hourly" + CommandNameThreeDay = "three-day" + CommandNameWeekend = "weekend" + CommandNameStorm = "storm" + + BatchNameMorning = "morning" + BatchNameEvening = "evening" +) + +func IDForCommandName(name string) (ID, error) { + switch name { + case CommandNameDaily: + return DailyToday, nil + case CommandNameTomorrow: + return Tomorrow, nil + case CommandNameHourly: + return Hourly, nil + case CommandNameThreeDay: + return ThreeDay, nil + case CommandNameWeekend: + return Weekend, nil + case CommandNameStorm: + return Storm, nil + default: + return "", fmt.Errorf("unknown report command %q", name) + } +} + +func CommandNames() []string { + return []string{ + CommandNameDaily, + CommandNameTomorrow, + CommandNameHourly, + CommandNameThreeDay, + CommandNameWeekend, + CommandNameStorm, + } +} + +func IDForConfigKey(key string) (ID, error) { + normalized := strings.ReplaceAll(strings.TrimSpace(strings.ToLower(key)), "-", "_") + switch normalized { + case "daily", "daily_today": + return DailyToday, nil + case "tomorrow": + return Tomorrow, nil + case "hourly": + return Hourly, nil + case "three_day", "three_day_outlook": + return ThreeDay, nil + case "weekend", "weekend_outlook": + return Weekend, nil + case "storm", "storm_report": + return Storm, nil + default: + return "", fmt.Errorf("report config key %q is not a known report", key) + } +} + +func BatchForCommandName(name string) (Batch, error) { + switch name { + case BatchNameMorning: + return Morning, nil + case BatchNameEvening: + return Evening, nil + default: + return "", fmt.Errorf("unknown batch command %q", name) + } +} + +func BatchCommandNames() []string { + return []string{ + BatchNameMorning, + BatchNameEvening, + } +} diff --git a/internal/report/period_test.go b/internal/report/period_test.go index e3ae1db..03be4ae 100644 --- a/internal/report/period_test.go +++ b/internal/report/period_test.go @@ -240,6 +240,113 @@ func TestBatchesDoNotIncludeHourly(t *testing.T) { } } +func TestIDForCommandName(t *testing.T) { + tests := []struct { + name string + want ID + }{ + {name: "daily", want: DailyToday}, + {name: "tomorrow", want: Tomorrow}, + {name: "hourly", want: Hourly}, + {name: "three-day", want: ThreeDay}, + {name: "weekend", want: Weekend}, + {name: "storm", want: Storm}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := IDForCommandName(tt.name) + if err != nil { + t.Fatalf("IDForCommandName() error = %v", err) + } + if got != tt.want { + t.Fatalf("IDForCommandName() = %q, want %q", got, tt.want) + } + }) + } + if names := strings.Join(CommandNames(), ","); names != "daily,tomorrow,hourly,three-day,weekend,storm" { + t.Fatalf("CommandNames() = %s, want stable command names", names) + } + if _, err := IDForCommandName("near-term"); err == nil || !strings.Contains(err.Error(), `unknown report command "near-term"`) { + t.Fatalf("IDForCommandName(near-term) error = %v, want unknown command", err) + } +} + +func TestIDForConfigKey(t *testing.T) { + tests := []struct { + key string + want ID + }{ + {key: "daily", want: DailyToday}, + {key: "daily_today", want: DailyToday}, + {key: "tomorrow", want: Tomorrow}, + {key: "hourly", want: Hourly}, + {key: "three_day", want: ThreeDay}, + {key: "three-day", want: ThreeDay}, + {key: "three_day_outlook", want: ThreeDay}, + {key: "three-day-outlook", want: ThreeDay}, + {key: "weekend", want: Weekend}, + {key: "weekend_outlook", want: Weekend}, + {key: "storm", want: Storm}, + {key: "storm_report", want: Storm}, + } + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + got, err := IDForConfigKey(tt.key) + if err != nil { + t.Fatalf("IDForConfigKey() error = %v", err) + } + if got != tt.want { + t.Fatalf("IDForConfigKey() = %q, want %q", got, tt.want) + } + }) + } + if _, err := IDForConfigKey("daily_tomorrow"); err == nil || !strings.Contains(err.Error(), `report config key "daily_tomorrow" is not a known report`) { + t.Fatalf("IDForConfigKey(daily_tomorrow) error = %v, want unknown key", err) + } +} + +func TestBatchForCommandName(t *testing.T) { + tests := []struct { + name string + want Batch + }{ + {name: "morning", want: Morning}, + {name: "evening", want: Evening}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := BatchForCommandName(tt.name) + if err != nil { + t.Fatalf("BatchForCommandName() error = %v", err) + } + if got != tt.want { + t.Fatalf("BatchForCommandName() = %q, want %q", got, tt.want) + } + }) + } + if names := strings.Join(BatchCommandNames(), ","); names != "morning,evening" { + t.Fatalf("BatchCommandNames() = %s, want stable batch command names", names) + } + if _, err := BatchForCommandName("hourly"); err == nil || !strings.Contains(err.Error(), `unknown batch command "hourly"`) { + t.Fatalf("BatchForCommandName(hourly) error = %v, want unknown batch", err) + } +} + +func TestMorningBatchReportOrder(t *testing.T) { + location := mustLoadLocation(t) + resolved, err := DefaultRegistry().BatchReports(Morning, ResolveRequest{ + Now: mustParse("2026-05-29T06:00:00-05:00"), + Location: location, + }) + if err != nil { + t.Fatalf("BatchReports() error = %v", err) + } + ids := resolvedIDs(resolved) + if strings.Join(ids, ",") != "daily_today,three_day,weekend" { + t.Fatalf("ids = %v, want morning report order", ids) + } +} + func TestRegistryLookupErrorIsActionable(t *testing.T) { _, err := DefaultRegistry().Lookup(ID("unknown")) if err == nil {