diff --git a/docs/roadmap/distributor-paths.md b/docs/roadmap/distributor-paths.md deleted file mode 100644 index b80389a..0000000 --- a/docs/roadmap/distributor-paths.md +++ /dev/null @@ -1,126 +0,0 @@ -# Distributor Report Paths Roadmap - -## Purpose - -This roadmap defines the target behavior for naming Markdown reports inside -distributor source bundles. - -Weatherreporter currently uses one application-level -`notify.distributor.report_path_templates` list for every report type. That -model is too coarse for the intended output layout because different reports -need different archive paths, latest paths, and report-specific aliases. - -## Locked Decisions - -- Remove `notify.distributor.report_path_templates`; do not keep a legacy - fallback or compatibility alias. -- Report definitions own canonical distributor output path templates. -- Configuration may override distributor output path templates per report. -- The app layer resolves report-specific distributor bundle paths before - calling the distributor adapter. -- The distributor adapter continues to receive explicit source-to-bundle file - mappings and does not choose report types, render templates, scan - workspaces, or apply report routing policy. -- Managed Markdown report files remain the only distributor upload sources. - Optional local output copies are not uploaded. -- Batch distributor uploads, when enabled, use the same report-specific bundle - path resolver for each included report. -- Non-roadmap documentation must not describe this behavior until it is - implemented. - -## Target Configuration Model - -The global path template list is removed from `notify.distributor`: - -```yaml -notify: - distributor: - enabled: false - endpoint: https://distributor.example.com - token_env: DISTRIBUTOR_UPLOAD_TOKEN - timeout: 30s - failure_policy: error - pipeline_id_template: "weatherreporter.{report_id}" - bundle_id_template: "weatherreporter.{location_id}.{report_id}" - idempotency_key_template: "{bundle_id}.{run_id}" -``` - -Per-report overrides live under `reports..distributor`: - -```yaml -reports: - daily: - distributor: - path_templates: - - "daily/{valid_start_date}/{run_id}.md" - - "daily/{valid_start_date}/index.md" -``` - -If a report override is omitted, weatherreporter uses the defaults declared by -that report definition. - -## Default Report Paths - -Each generated report maps its managed Markdown source file to one or more -bundle-relative distributor paths. - -| Report | Default distributor paths | -| --- | --- | -| `hourly` | `hourly/index.md` | -| `daily` | `daily/{valid_start_date}/{run_id}.md`; `daily/{valid_start_date}/index.md` | -| `today` | `daily/{valid_start_date}/{run_id}.md`; `daily/{valid_start_date}/index.md`; `today/index.md` | -| `tomorrow` | `daily/{valid_start_date}/{run_id}.md`; `daily/{valid_start_date}/index.md`; `tomorrow/index.md` | -| `three_day` | `three-day/{valid_start_date}/{run_id}.md`; `three-day/{valid_start_date}/index.md` | -| `weekend` | `weekend/{valid_start_date}/{run_id}.md`; `weekend/{valid_start_date}/index.md` | -| `storm` | `storm/{storm_id}/{run_id}.md`; `storm/{storm_id}/index.md` | - -`storm_id` is derived from the resolved storm valid period until a future -explicit storm identity source exists. - -## Template Values - -Report path templates keep the existing report template values: - -- `location_id` -- `report_id` -- `run_id` -- `artifact_group` -- `batch_output_name` -- `valid_start_date` -- `valid_end_date` -- `valid_start_time` -- `valid_end_time` -- `valid_start_stamp` -- `valid_end_stamp` -- `storm_id` - -The initial `storm_id` value is `{valid_start_stamp}-{valid_end_stamp}` in the -effective report timezone. It is available for all single-report distributor -templates, but it renders as an empty value for non-storm reports. - -Rendered paths must stay unique relative paths using `/` separators. They must -not contain backslashes, empty path segments, `.`, `..`, `manifest.json`, or -`.distributor.json`. - -## Intended Final State - -`internal/report.Definition` declares distributor path defaults alongside the -other report-owned behavior such as report ID, prompt ID, valid-period -resolution, module composition, comparison strategy, artifact group, and output -name. - -`internal/config` owns per-report override loading and validation. Unknown -fields under report config continue to fail validation or YAML unmarshalling. - -`internal/app` resolves bundle paths through one helper used by both -single-report notification and batch notification. That helper applies this -precedence: - -1. `reports..distributor.path_templates`, when explicitly configured. -2. `report.Definition` distributor path defaults. - -There is no third global fallback. - -The distributor adapter API remains unchanged unless a separate adapter concern -requires it. It should still upload the explicit file mappings passed by the -app layer. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index 77400d3..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,288 +0,0 @@ -# Distributor Paths Implementation Plan - -## Purpose - -This document is the staged implementation plan for -[distributor-paths.md](distributor-paths.md). It is written for an LLM coding -agent that will implement the feature in order. - -The feature is complete only when `notify.distributor.report_path_templates` is -removed, report definitions provide distributor path defaults, per-report -config overrides work, and single-report plus batch distributor notifications -resolve bundle paths through the same report-aware code path. - -## Ground Rules - -- Review `docs/policy/architecture.md`, `docs/policy/development.md`, and - `docs/policy/documentation.md` before editing code. -- Keep distributor package types confined to `internal/adapters/distributor`. -- Do not add a compatibility fallback for - `notify.distributor.report_path_templates`. -- Do not upload optional `--out` or `--out-dir` copies to distributor. -- Keep all non-roadmap documentation changes in the stage that implements the - behavior being documented. -- Prefer small helpers in existing packages over a new package unless a clear - boundary emerges during implementation. - -## Decisions Applied - -- Per-report override key: use - `reports..distributor.path_templates`. -- `three_day` and `weekend` get defaults now: - `three-day/{valid_start_date}/{run_id}.md`, - `three-day/{valid_start_date}/index.md`, - `weekend/{valid_start_date}/{run_id}.md`, and - `weekend/{valid_start_date}/index.md`. -- Initial `storm_id`: derive from the resolved valid period as - `{valid_start_stamp}-{valid_end_stamp}` in the effective report timezone. - This avoids adding new CLI or upstream API requirements while still producing - stable storm paths for the same storm window. -- Allow `storm_id` in all single-report distributor templates. It renders empty - for non-storm reports. - -## Stage 1: Report Defaults - -Goal: make distributor path defaults part of report definitions without -changing runtime behavior yet. - -Code changes: - -- Add `DistributorPathTemplates []string` to `internal/report.Definition`. -- Populate defaults in every generated report definition: - `daily`, `today`, `tomorrow`, `hourly`, `three_day`, `weekend`, and `storm`. -- Ensure registry cloning preserves `DistributorPathTemplates` when module - overrides are applied. -- Keep this field report-owned; do not reference distributor adapter types from - `internal/report`. - -Tests: - -- Add focused report tests that every generated report has at least one - distributor path template. -- Add table coverage for the exact default templates listed in - `docs/roadmap/distributor-paths.md`. -- Run: - -```bash -go test ./internal/report -``` - -Completion criteria: - -- Report definitions declare all default distributor bundle paths. -- No app behavior changes are required in this stage. - -## Stage 2: Config Override Model - -Goal: add per-report distributor path override configuration while preserving -the existing runtime path until the app layer is switched in Stage 3. - -Code changes: - -- Add `ReportDistributorConfig` under `internal/config`. -- Add `Distributor ReportDistributorConfig` to `ReportConfig`. -- Support only this YAML shape: - -```yaml -reports: - daily: - distributor: - path_templates: - - "daily/{valid_start_date}/{run_id}.md" -``` - -- Track whether `path_templates` was explicitly set so omitted overrides can - fall back to report definition defaults. -- Reject unknown fields under both `reports.` and - `reports..distributor`. -- Add a config helper that returns normalized overrides by `report.ID`, reusing - `report.IDForConfigKey` and duplicate report-key detection. -- Validate configured path templates with the same parser and path safety rules - used for rendered distributor paths. - -Tests: - -- Add config load/unmarshal tests for per-report distributor overrides. -- Add validation tests for unknown fields, duplicate report aliases, unknown - template variables, absolute paths, `..`, `manifest.json`, duplicate rendered - paths inside one report override, and an explicitly empty override list. -- Run: - -```bash -go test ./internal/config -``` - -Completion criteria: - -- Config can express per-report distributor path overrides. -- Omitted overrides are distinguishable from explicit empty lists. -- No runtime notification path selection has been switched yet. - -## Stage 3: Template Rendering - -Goal: generalize distributor template rendering so it no longer hardcodes the -old global config field name and can render `storm_id`. - -Code changes: - -- Add `StormID string` to `config.DistributorTemplateValues`. -- Add `storm_id` to the allowed variables for single-report distributor - templates. -- Derive `StormID` in the app-layer template-value builder as - `{valid_start_stamp}-{valid_end_stamp}` for `report.Storm`; leave it empty - for other reports. -- Replace hardcoded error names such as - `notify.distributor.report_path_templates[0]` with caller-provided names such - as `reports.daily.distributor.path_templates[0]` or - `report.daily.distributor_path_templates[0]`. -- Keep rendered path validation in `internal/config` unless the implementation - reveals a cleaner existing boundary. - -Tests: - -- Add rendering tests for `storm_id`. -- Update existing rendering tests so error messages reference the new caller - names rather than the removed global config field. -- Confirm duplicate path detection still reports the duplicate path. -- Run: - -```bash -go test ./internal/config ./internal/app -``` - -Completion criteria: - -- Rendering supports all variables in the feature roadmap. -- Rendering helpers can be used for both defaults and per-report overrides - without naming errors after the removed global field. - -## Stage 4: App Notification Path Resolver - -Goal: switch single-report and batch distributor notification to the -report-specific path resolver. - -Code changes: - -- Add one app-layer helper used by both `buildNotificationRequest` and - `buildBatchNotificationRequest`. -- Helper precedence: - 1. explicit `reports..distributor.path_templates`; - 2. `resolved.Definition.DistributorPathTemplates`. -- Return an actionable error if a report has neither an override nor defaults. -- Preserve existing single-report identity rendering for `pipeline_id_template`, - `bundle_id_template`, and `idempotency_key_template`. -- Preserve existing batch identity rendering under `notify.distributor.batch`. -- Preserve batch duplicate detection across all rendered bundle paths before - calling distributor. -- Ensure errors include report ID, RunID, source path where available, and the - rendered bundle path when relevant. - -Tests: - -- Update single-report notification tests for default paths: - `hourly`, `daily`, `today`, `tomorrow`, `three_day`, `weekend`, and `storm` - where storm generation is currently testable. -- Add per-report override precedence tests. -- Add batch tests proving each included report uses its own defaults or - overrides. -- Keep or add a batch duplicate-path test. The current planned batches avoid - `today`/`tomorrow` collisions with future dated `daily` reports, but the - collision guard must remain explicit for future batch changes. -- Confirm notification source paths are still managed Markdown report paths, - not output copies. -- Run: - -```bash -go test ./internal/app -``` - -Completion criteria: - -- Distributor upload requests contain report-specific bundle paths. -- Single-report and batch notifications use the same path resolution rules. -- No distributor adapter API change is required. - -## Stage 5: Remove The Legacy Global Field - -Goal: hard-remove `notify.distributor.report_path_templates` from the codebase. - -Code changes: - -- Remove `ReportPathTemplates` from `DistributorNotifyConfig`. -- Remove its default from `internal/config/defaults.go`. -- Remove validation that requires or renders the global field. -- Add or update `DistributorNotifyConfig.UnmarshalYAML` so unknown fields in - `notify.distributor` fail during config parsing. This must explicitly reject - the removed `report_path_templates` key instead of silently ignoring it. -- Update or remove tests that asserted the old global default. -- Search for and remove remaining code references: - -```bash -rg "ReportPathTemplates|report_path_templates" -``` - -Tests: - -- Add or update config tests proving the global field is no longer accepted. -- Run: - -```bash -go test ./internal/config ./internal/app -``` - -Completion criteria: - -- The legacy global path field is gone from structs, defaults, validation, - examples, docs, and tests. -- There is no compatibility fallback. - -## Stage 6: Documentation And Examples - -Goal: move implemented behavior from roadmap-only docs into maintained user and -internal docs. - -Documentation changes: - -- Update `docs/config.md`: - - remove `notify.distributor.report_path_templates`; - - document `reports..distributor.path_templates`; - - document default path behavior and template variables; - - document `storm_id` derivation. -- Update `docs/internal/distributor-adapter.md` so it says the app layer - resolves report-specific path templates before calling the adapter. -- Update `examples/config.yml` to remove the old global field and optionally - include one concise per-report override example if useful. -- Do not add unimplemented behavior outside `docs/roadmap/`. - -Tests and checks: - -```bash -go test ./internal/config ./internal/app ./internal/adapters/distributor -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -Completion criteria: - -- Maintained docs and examples match implemented behavior. -- Example config still loads in the config test suite. - -## Final Verification - -Before considering the feature complete, run: - -```bash -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -rg "ReportPathTemplates|report_path_templates" --glob '!docs/roadmap/**' -``` - -The final `rg` should find no implemented-code, maintained-doc, or example -references to the removed global field. - -## Open Questions - -None. The prior roadmap questions are resolved above so the implementation can -proceed without additional product decisions. diff --git a/internal/config/config.go b/internal/config/config.go index 9f60bd5..f2acb0a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -202,6 +202,37 @@ func (c *DistributorNotifyConfig) UnmarshalYAML(value *yaml.Node) error { return nil } +func (c *DistributorBatchNotifyConfig) UnmarshalYAML(value *yaml.Node) error { + if value.Kind != yaml.MappingNode { + return fmt.Errorf("notify distributor batch entry must be a mapping") + } + for i := 0; i < len(value.Content); i += 2 { + key := value.Content[i].Value + node := value.Content[i+1] + switch key { + case "enabled": + if err := node.Decode(&c.Enabled); err != nil { + return err + } + case "pipeline_id_template": + if err := node.Decode(&c.PipelineIDTemplate); err != nil { + return err + } + case "bundle_id_template": + if err := node.Decode(&c.BundleIDTemplate); err != nil { + return err + } + case "idempotency_key_template": + if err := node.Decode(&c.IdempotencyKeyTemplate); err != nil { + return err + } + default: + return fmt.Errorf("unknown notify distributor batch field %q", key) + } + } + return nil +} + func (c *ReportDistributorConfig) UnmarshalYAML(value *yaml.Node) error { if value.Kind != yaml.MappingNode { return fmt.Errorf("report distributor entry must be a mapping") diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8892746..7c3d95a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -751,6 +751,17 @@ reports: `, wantErr: `reports.daily.distributor.path_templates renders duplicate path "daily/index.md"`, }, + { + name: "NonStormStormIDEmptyPathSegment", + yaml: ` +reports: + daily: + distributor: + path_templates: + - "daily/{storm_id}/index.md" +`, + wantErr: "reports.daily.distributor.path_templates[0] must not render empty path segments", + }, { name: "EmptyOverrideList", yaml: ` @@ -776,6 +787,23 @@ reports: } } +func TestReportDistributorPathOverrideStormIDValidation(t *testing.T) { + _, err := LoadFile(writeConfig(t, ` +reports: + daily: + distributor: + path_templates: + - "daily/storm-{storm_id}.md" + storm: + distributor: + path_templates: + - "storm/{storm_id}/index.md" +`)) + if err != nil { + t.Fatalf("LoadFile() error = %v", err) + } +} + func TestReportDistributorPathOverridesConsistentForLoadedAndConstructedConfig(t *testing.T) { yaml := ` reports: @@ -1087,6 +1115,46 @@ notify: } } +func TestDistributorBatchNotifyRejectsUnknownFields(t *testing.T) { + _, err := LoadFile(writeConfig(t, ` +notify: + distributor: + batch: + paths: + - index.md +`)) + if err == nil { + t.Fatal("LoadFile() error = nil, want unknown distributor batch field error") + } + if !strings.Contains(err.Error(), `unknown notify distributor batch field "paths"`) { + t.Fatalf("error = %q, want unknown batch field rejection", err.Error()) + } +} + +func TestDistributorBatchNotifyPartialConfigPreservesDefaults(t *testing.T) { + cfg, err := LoadFile(writeConfig(t, ` +notify: + distributor: + batch: + enabled: false +`)) + if err != nil { + t.Fatalf("LoadFile() error = %v", err) + } + if cfg.Notify.Distributor.Batch.Enabled { + t.Fatalf("Batch.Enabled = true, want false") + } + if cfg.Notify.Distributor.Batch.PipelineIDTemplate != "weatherreporter" { + t.Fatalf("Batch.PipelineIDTemplate = %q, want default", cfg.Notify.Distributor.Batch.PipelineIDTemplate) + } + if cfg.Notify.Distributor.Batch.BundleIDTemplate != "weatherreporter.{location_id}.{batch}" { + t.Fatalf("Batch.BundleIDTemplate = %q, want default", cfg.Notify.Distributor.Batch.BundleIDTemplate) + } + if cfg.Notify.Distributor.Batch.IdempotencyKeyTemplate != "{bundle_id}.{batch_run_id}" { + t.Fatalf("Batch.IdempotencyKeyTemplate = %q, want default", cfg.Notify.Distributor.Batch.IdempotencyKeyTemplate) + } +} + func TestDisabledDistributorNotifyAcceptsMalformedBatchTemplates(t *testing.T) { cfg := Defaults() cfg.Notify.Distributor.Enabled = false diff --git a/internal/config/reports.go b/internal/config/reports.go index 60bf66c..9ca4d32 100644 --- a/internal/config/reports.go +++ b/internal/config/reports.go @@ -113,7 +113,7 @@ func traverseReportDistributorPathOverrides(cfg Config) (map[report.ID][]string, if !reportCfg.Distributor.pathTemplatesSet { continue } - if err := validateReportDistributorPathTemplates(key, reportCfg.Distributor.PathTemplates); err != nil { + if err := validateReportDistributorPathTemplates(key, reportID, reportCfg.Distributor.PathTemplates); err != nil { return nil, err } overrides[reportID] = append([]string(nil), reportCfg.Distributor.PathTemplates...) @@ -121,13 +121,21 @@ func traverseReportDistributorPathOverrides(cfg Config) (map[report.ID][]string, return overrides, nil } -func validateReportDistributorPathTemplates(reportKey string, templates []string) error { +func validateReportDistributorPathTemplates(reportKey string, reportID report.ID, templates []string) error { name := fmt.Sprintf("reports.%s.distributor.path_templates", reportKey) - _, err := RenderDistributorReportPaths(name, templates, sampleDistributorTemplateValues()) + _, err := RenderDistributorReportPaths(name, templates, sampleDistributorTemplateValuesForReport(reportID)) return err } func sampleDistributorTemplateValues() DistributorTemplateValues { + return sampleDistributorTemplateValuesForReport(report.Storm) +} + +func sampleDistributorTemplateValuesForReport(reportID report.ID) DistributorTemplateValues { + stormID := "" + if reportID == report.Storm { + stormID = "2026-05-29T0000-2026-05-30T0000" + } return DistributorTemplateValues{ LocationID: "location", ReportID: "report", @@ -140,7 +148,7 @@ func sampleDistributorTemplateValues() DistributorTemplateValues { ValidEndTime: "0000", ValidStartStamp: "2026-05-29T0000", ValidEndStamp: "2026-05-30T0000", - StormID: "2026-05-29T0000-2026-05-30T0000", + StormID: stormID, } }