eric/weatherreporter
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Finalize and close the distributor report path refactor roadmap

Browse Source
This commit is contained in:
Eric Rakestraw
2026-06-20 07:41:37 -05:00
parent 15ee4af1a1
commit b8e889ad13
5 changed files with 111 additions and 418 deletions
Download Patch File Download Diff File Expand all files Collapse all files

126
docs/roadmap/distributor-paths.md
View File

@@ -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.<report>.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.<report>.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.

288
docs/roadmap/implementation.md
View File

@@ -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.<report>.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.<report>` and
`reports.<report>.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.<report>.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.<report>.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.

31
internal/config/config.go
View File

@@ -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")

68
internal/config/config_test.go
View File

@@ -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

16
internal/config/reports.go
View File

@@ -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,
}
}