From 7adf5e1b082619b764a8e850e917518e9ef9c790 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 19 Jun 2026 21:38:30 -0500 Subject: [PATCH] Add a feature roadmap and implementation plan to refactor configuration for distributor output paths --- docs/roadmap/batch-distributor.md | 360 ----------------- docs/roadmap/distributor-paths.md | 126 ++++++ docs/roadmap/implementation.md | 619 +++++++++++------------------- 3 files changed, 348 insertions(+), 757 deletions(-) delete mode 100644 docs/roadmap/batch-distributor.md create mode 100644 docs/roadmap/distributor-paths.md diff --git a/docs/roadmap/batch-distributor.md b/docs/roadmap/batch-distributor.md deleted file mode 100644 index 9a42a95..0000000 --- a/docs/roadmap/batch-distributor.md +++ /dev/null @@ -1,360 +0,0 @@ -# Batch Distributor Upload Roadmap - -## Purpose - -This roadmap defines the target behavior for distributor notification from -`weatherreporter run morning` and `weatherreporter run evening`. - -Batch commands should publish the generated weather reports as one coherent -distributor source bundle. The batch upload should be atomic from -weatherreporter's point of view: either every planned report is successfully -generated and the whole bundle is uploaded, or distributor is not notified. - -Single-report `weatherreporter generate ...` commands should keep the existing -per-report distributor notification behavior. - -## Current Problem - -Per-report uploads in one batch can create multiple distributor source bundles -with the same logical destination scope. Depending on configured bundle IDs, -created timestamps, distributor merge mode, and destination state, distributor -may reject later uploads as conflicting with an earlier bundle from the same -batch. - -The motivating failure was: - -```text -destination source has same id and created time but different digest -``` - -Unique per-report RunIDs reduce one collision class, but they do not provide -batch-level publication semantics. A batch-level upload is the cleaner -long-term boundary when the downstream pipeline is intended to publish a -coherent set of weatherreporter outputs. - -## Target Behavior - -- `generate ` uploads one distributor bundle for that single generated - report when distributor notification is enabled. -- `run morning` and `run evening` generate all planned reports first. -- If collection, planning, report generation, Scriptorium, template rendering, - metadata persistence, or output-copy persistence fails for any report, the - batch distributor upload is skipped for the whole batch. -- If every planned report succeeds, weatherreporter uploads one distributor - bundle containing every managed Markdown report from the batch. -- The batch bundle maps each managed Markdown source path to one or more - bundle-relative paths rendered from the report's definition, valid period, - RunID, and configured distributor path templates. -- Batch upload failure marks the batch as failed and makes the command return a - nonzero aggregate error, but it does not retroactively mark individual report - generation as failed. -- Distributor status polling and debug artifact capture remain required for the - batch upload, just as they are for per-report uploads today. -- Optional `--out-dir` copies remain operator conveniences and are never used as - distributor upload sources. - -## Locked Decisions - -- Batch notification is all-or-nothing. If one report fails, skip the - distributor upload for the entire batch. -- The initial batch upload uses one distributor pipeline for the whole batch. -- Do not implement report-type-specific routing inside a single batch upload. -- Keep distributor package types confined to `internal/adapters/distributor`. -- Keep single-report notification behavior in place for `generate` commands. -- Do not implement batch-level publication by asking distributor to scan the - weatherreporter workspace. -- Do not introduce a manifest/resume system, workflow engine, plugin system, or - distributor-specific CLI flags. - -## Batch Artifact Path - -Batch notification should have its own debug artifact path rather than being -attached to one arbitrary report. - -Target layout: - -```text -workspace/ - notifications/ - batches/ - morning/ - 2026-06-17/ - 20260617T120000.123456789Z_morning.distributor.json - evening/ - 2026-06-17/ - 20260617T230000.123456789Z_evening.distributor.json -``` - -The batch notification artifact should include: - -- schema version; -- batch kind; -- batch run ID; -- attempted time; -- endpoint; -- pipeline ID; -- bundle ID; -- idempotency key; -- bundle created timestamp; -- included report records, each with report ID, RunID, managed source path, and - rendered bundle paths; -- accepted distributor run ID and upload status; -- latest distributor run status; -- raw distributor run report JSON when available; -- status lookup error when available; -- redacted notification error when upload or downstream run processing fails. - -This makes batch notification a first-class batch artifact and keeps future -debugging or inspection behavior straightforward. - -## Batch JSON Shape - -Batch notification should be represented as a top-level object on `BatchResult`. - -Target shape: - -```json -{ - "batch": "evening", - "startedAt": "2026-06-17T18:50:37.642224552-05:00", - "finishedAt": "2026-06-17T18:51:04.000000000-05:00", - "total": 3, - "succeeded": 3, - "failed": 0, - "notification": { - "status": "succeeded", - "runId": "weatherreporter.20260617T235104Z.77d8f75f", - "pipelineId": "weatherreporter", - "bundleId": "weatherreporter.home.evening", - "idempotencyKey": "weatherreporter.home.evening.20260617T235037.642224552Z_evening", - "path": "/var/lib/weatherreporter/notifications/batches/evening/2026-06-17/20260617T235037.642224552Z_evening.distributor.json", - "includedReports": [ - { - "reportId": "tomorrow", - "runId": "20260617T235037.642224552Z_tomorrow", - "sourcePath": "/var/lib/weatherreporter/reports/tomorrow/20260617T235037.642224552Z_tomorrow.md", - "bundlePaths": ["tomorrow/index.md"] - }, - { - "reportId": "daily", - "runId": "20260617T235037.642224552Z_daily_2026-06-23", - "sourcePath": "/var/lib/weatherreporter/reports/daily/20260617T235037.642224552Z_daily_2026-06-23.md", - "bundlePaths": ["archive/2026-06-23/daily/20260617T235037.642224552Z_daily_2026-06-23.md"] - } - ] - }, - "reports": [] -} -``` - -When distributor is disabled, omit the notification object. When notification is -skipped because a report failed, include: - -```json -"notification": { - "status": "skipped", - "reason": "one or more reports failed" -} -``` - -Do not duplicate the same batch notification status into every report item. -Per-report notification fields should remain meaningful for single-report -notification paths. - -## Configuration Model - -Use the existing `notify.distributor` block for both single-report and batch -notification, but add batch-specific identity templates so operators can keep -single-report and batch source identity separate. - -Target config shape: - -```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}" - report_path_templates: - - "{valid_start_date}/{artifact_group}/{valid_start_date}-{artifact_group}-{run_id}.md" - batch: - enabled: true - pipeline_id_template: "weatherreporter" - bundle_id_template: "weatherreporter.{location_id}.{batch}" - idempotency_key_template: "{bundle_id}.{batch_run_id}" -``` - -Rules: - -- `notify.distributor.enabled=false` disables both single-report and batch - notification. -- `notify.distributor.batch.enabled=true` makes run commands use the new - batch-level upload behavior. -- `notify.distributor.batch.enabled=false` disables distributor notification - for run commands. It does not fall back to legacy per-report batch uploads. -- Existing report-level `report_path_templates` are reused to map each report - source into the batch bundle. -- Each report path template is rendered once per included report using that - report's normal distributor template values. -- Batch templates support at least `location_id`, `batch`, `batch_run_id`, and - `batch_started_date`. -- Batch idempotency templates may also use `bundle_id`. -- Batch-level `pipeline_id_template` does not use `report_id` because one - upload contains multiple reports. -- Batch bundle IDs are stable for a logical batch stream. The idempotency key, - not the bundle ID, identifies a specific retryable batch attempt. - -## Batch Run Identity - -Batch notification needs an app-level batch run ID for notification identity and -artifact naming. - -Format: - -```text -_ -``` - -Examples: - -```text -20260617T235037.642224552Z_morning -20260617T235037.642224552Z_evening -``` - -The timestamp is based on batch `StartedAt` in UTC with the same nanosecond -layout used by report RunIDs. This ID is separate from report RunIDs and should -not replace them. - -## Bundle Construction - -Batch uploads use one distributor adapter upload request with multiple file -mappings. - -For each successful report: - -1. Use `BatchReportResult.ReportPath` as the source path. -2. Render `notify.distributor.report_path_templates` using that report's - existing distributor template values. -3. Add one upload file mapping per rendered bundle path. - -Validation should fail before upload if: - -- no reports are included; -- any included report lacks a managed report path; -- any source path is empty; -- any bundle path is empty or invalid; -- two rendered bundle paths in the same batch collide. - -Errors should include report ID, RunID, source path, and bundle path context -where useful. Token values must never appear in errors, JSON output, metadata, -or artifacts. - -## App Workflow - -Batch commands should still collect once, plan once, and generate reports -through the same report generation path used by single-report commands. The -batch workflow differs only at the notification boundary: - -- per-report distributor notification is suppressed for `run morning` and - `run evening`; -- report generation continues after individual report failures, preserving - current batch behavior; -- batch notification is skipped if any report failed; -- one batch notification is attempted only when all planned reports succeeded; -- batch notification failure produces an aggregate batch error and a batch - notification debug artifact. - -Suppressing per-report notification should be explicit app-layer behavior, not a -hidden side effect inside `GenerateReport`. - -## Adapter Boundary - -The existing distributor adapter already supports multiple file mappings through -its upload request. Batch uploads should reuse that adapter API rather than -introducing a second distributor client. - -The app layer owns: - -- batch report selection; -- batch upload identity rendering; -- source path selection; -- bundle path rendering; -- debug artifact persistence; -- batch JSON shape. - -The adapter owns: - -- distributor package type conversion; -- token lookup; -- upload client construction; -- upload request execution; -- status polling; -- upload/status error wrapping and redaction. - -## State And Inspection - -State should support saving batch notification artifacts without introducing a -full manifest/resume system. - -Expected additions: - -- batch-level artifact path calculation for distributor notification; -- save support for the batch distributor notification artifact; -- no required changes to existing report metadata artifact shape. - -If linking is useful later, prefer adding the batch notification path to the -top-level batch JSON result first, then consider metadata links. - -## CLI Output - -Command syntax remains unchanged: - -```text -weatherreporter run morning -weatherreporter run evening -``` - -Stderr should keep one compact line per report generation result, then add one -compact batch notification line when notification is attempted, skipped, or -failed. The same notification error should not be repeated on every report line. - -Example: - -```text -report=tomorrow status=succeeded output="" -report=daily status=succeeded output="" -batchNotification status=succeeded runId="weatherreporter.20260617T235104Z.77d8f75f" -batch=evening total=2 succeeded=2 failed=0 -``` - -If report generation fails: - -```text -report=daily status=failed error="..." -batchNotification status=skipped reason="one or more reports failed" -batch=evening total=2 succeeded=1 failed=1 -``` - -## Documentation Boundary - -Until implemented, this roadmap is the only place this behavior should be -described. After implementation, update the relevant implemented docs and keep -future extensions under `docs/roadmap/`. - -## Deferred Work - -- Batch-level durable resume or retry queues. -- Batch-level distributor status inspection command. -- Multiple distributor pipelines within one batch. -- Report-type-specific distributor routing inside batch uploads. -- Uploading data packages, metadata, render contexts, or notification artifacts - in the batch bundle. -- Distributor-specific CLI flags. -- Batch manifest or progress system. -- Changing distributor destination merge semantics from weatherreporter. diff --git a/docs/roadmap/distributor-paths.md b/docs/roadmap/distributor-paths.md new file mode 100644 index 0000000..b80389a --- /dev/null +++ b/docs/roadmap/distributor-paths.md @@ -0,0 +1,126 @@ +# 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 index 2d019c2..77400d3 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,463 +1,288 @@ -# Batch Distributor Upload Implementation Roadmap +# Distributor Paths Implementation Plan ## Purpose -Implement the batch distributor upload behavior defined in -`docs/roadmap/batch-distributor.md`. +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. -This plan is written for an LLM coding agent. Implement the stages in order. -Preserve existing public CLI syntax, report generation behavior, managed report -paths, and single-report distributor notification behavior unless a stage -explicitly changes batch notification behavior. +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. -## Source Roadmap +## Ground Rules -Use `docs/roadmap/batch-distributor.md` as the authoritative source for: +- 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. -- user intent; -- locked policy decisions; -- target batch JSON shape; -- batch notification artifact layout; -- configuration shape; -- app, state, and adapter boundaries; -- deferred work. +## Decisions Applied -Do not move aspirational batch distributor behavior into non-roadmap docs until -the corresponding code is implemented. +- 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. -## Locked Decisions +## Stage 1: Report Defaults -- `generate ` keeps per-report distributor notification. -- `run morning` and `run evening` use one batch-level distributor upload when - distributor notification is enabled and batch notification is enabled. -- `notify.distributor.batch.enabled=false` disables distributor notification - for run commands; it does not fall back to per-report batch uploads. -- Batch upload is all-or-nothing: if any report fails, skip the distributor - upload for the whole batch. -- A batch notification failure makes the batch command return a nonzero - aggregate error, but it does not mark individual report generation as failed. -- The initial implementation uses one distributor pipeline for the whole batch. -- The batch upload source files are managed Markdown report paths only. - `--out-dir` copies are never uploaded. -- Reuse the existing distributor adapter; do not import distributor package - types outside `internal/adapters/distributor`. -- Do not introduce batch manifest/resume behavior, distributor-specific CLI - flags, a workflow engine, plugin system, or broad CLI redesign. +Goal: make distributor path defaults part of report definitions without +changing runtime behavior yet. -## Stage 1: Batch Notify Config And Template Rendering +Code changes: -Goal: add batch-specific distributor notification configuration and rendering -without changing runtime behavior yet. - -Implementation guidance: - -- Add a nested batch config under `notify.distributor`, for example: - ```go - type DistributorBatchNotifyConfig struct { - Enabled bool `yaml:"enabled"` - PipelineIDTemplate string `yaml:"pipeline_id_template"` - BundleIDTemplate string `yaml:"bundle_id_template"` - IdempotencyKeyTemplate string `yaml:"idempotency_key_template"` - } - ``` -- Add it to `config.DistributorNotifyConfig` as `Batch`. -- Defaults: - - `batch.enabled: true` - - `batch.pipeline_id_template: "weatherreporter"` - - `batch.bundle_id_template: "weatherreporter.{location_id}.{batch}"` - - `batch.idempotency_key_template: "{bundle_id}.{batch_run_id}"` -- Preserve existing report-level distributor defaults and validation. -- When `notify.distributor.enabled=false`, batch values may remain defaulted but - must not require endpoint/token availability. -- When `notify.distributor.enabled=true` and `batch.enabled=true`, validate: - - non-empty batch pipeline template; - - non-empty batch bundle template; - - non-empty batch idempotency template; - - known template variables only; - - rendered values are non-empty for a representative validation input. -- Supported batch template variables: - - `location_id` - - `batch` - - `batch_run_id` - - `batch_started_date` - - `bundle_id` for idempotency templates only -- Add config rendering helpers, for example: - - `RenderDistributorBatchBundleID` - - `RenderDistributorBatchPipelineID` - - `RenderDistributorBatchIdempotencyKey` -- Keep report-level render helpers unchanged. -- Update `examples/config.yml` only after the config fields are implemented. +- 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: -- Defaults include enabled batch config and expected templates. -- Disabled distributor config accepts default batch settings. -- Enabled distributor config rejects malformed batch templates. -- Unknown batch template variables are rejected. -- Batch idempotency templates can reference `{bundle_id}`. -- Example config loads and includes the batch block if updated in this stage. +- 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: -Run: +```bash +go test ./internal/report +``` -```sh +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 ``` -## Stage 2: Batch Notification State Artifacts +Completion criteria: -Goal: add batch-level notification artifact paths and persistence. +- 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. -Implementation guidance: +## Stage 3: Template Rendering -- Add a state-owned batch notification schema version, for example: - `weatherreporter.batch_distributor_notification.v1`. -- Add a `BatchDistributorNotificationArtifact` type with fields matching - `docs/roadmap/batch-distributor.md`: - - schema version; - - batch kind; - - batch run ID; - - attempted time; - - endpoint; - - pipeline ID; - - bundle ID; - - idempotency key; - - bundle created timestamp; - - included report records; - - status; - - upload result; - - run status; - - status error; - - error. -- Add included report record fields: - - report ID; - - RunID; - - source path; - - bundle paths. -- Add state path support for: - ```text - notifications/batches///.distributor.json - ``` -- Use the batch start date in the effective Weather API/report timezone for the - local date directory. -- Add a save method such as `SaveBatchDistributorNotification`. -- Use existing atomic JSON write helpers. -- Do not change existing per-report metadata JSON shape in this stage. +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: -- Batch notification paths use the configured workspace root. -- Morning and evening paths include batch kind, local date, and batch run ID. -- Saved artifact round-trips with included reports, upload result, run status, - raw report JSON, and error fields. -- Empty batch run ID or batch kind fails with actionable errors. +- 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: -Run: - -```sh -go test ./internal/state +```bash +go test ./internal/config ./internal/app ``` -## Stage 3: Batch Notification App Types And Identity +Completion criteria: -Goal: add app-owned batch notification request/result types and deterministic -batch run ID helpers. +- 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. -Implementation guidance: +## Stage 4: App Notification Path Resolver -- Add app-level types, for example: - ```go - type BatchNotificationResult struct { - Status string - Reason string - RunID string - PipelineID string - BundleID string - IdempotencyKey string - Path string - IncludedReports []BatchNotificationReport - Error string - } - ``` -- Add JSON tags matching the target `BatchResult.notification` shape. -- Add `Notification *BatchNotificationResult` to `BatchResult` with - `json:"notification,omitempty"`. -- Add `BatchNotificationReport` with report ID, RunID, source path, and bundle - paths. -- Add an unexported batch run ID helper: - ```text - _ - ``` - using the same UTC nanosecond timestamp layout as report RunIDs. -- Add unexported batch distributor template values and rendering glue in - `internal/app` or call the new `internal/config` helpers directly with a - narrow value struct. -- Do not call distributor in this stage. +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: -- Batch run ID for morning/evening uses UTC timestamp plus batch name. -- Batch result JSON omits `notification` when nil. -- Batch result JSON includes notification fields when populated. +- 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: -Run: - -```sh +```bash go test ./internal/app ``` -## Stage 4: Batch Bundle Request Construction +Completion criteria: -Goal: build a validated multi-file distributor notification request from a -successful batch result. +- 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. -Implementation guidance: +## Stage 5: Remove The Legacy Global Field -- Add an unexported app helper that accepts: - - config; - - batch kind; - - batch run ID; - - batch started time; - - successful batch report results; - - the matching resolved/planned reports if needed for valid-period template - values. -- Render batch `pipeline_id`, `bundle_id`, and `idempotency_key` from - `notify.distributor.batch`. -- For each included report, render existing report-level - `report_path_templates` using that report's normal distributor template - values. -- Build one app notification request with multiple file mappings. -- If the current `NotificationRequest` is too report-specific, introduce a - separate unexported batch request type and convert it to - `distributoradapter.UploadRequest` at the notifier boundary. -- Validate before upload: - - no reports included; - - missing source path; - - missing or invalid bundle path; - - duplicate rendered bundle paths across the batch; - - missing batch pipeline, bundle ID, or idempotency key. -- Error context should include report ID, RunID, source path, and bundle path - where useful. -- Do not include token values in errors. +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: -- Evening batch with Tomorrow plus two Daily reports renders one request with - all managed report paths. -- Every included report uses its own normal rendered report path templates. -- Duplicate bundle paths fail before upload. -- Missing report path fails before upload. -- Batch ID, bundle ID, pipeline ID, and idempotency key match configured - templates. +- Add or update config tests proving the global field is no longer accepted. +- Run: -Run: - -```sh -go test ./internal/app ./internal/config +```bash +go test ./internal/config ./internal/app ``` -## Stage 5: Suppress Per-Report Batch Notification +Completion criteria: -Goal: make run commands stop notifying once per report while preserving -single-report notification. +- The legacy global path field is gone from structs, defaults, validation, + examples, docs, and tests. +- There is no compatibility fallback. -Implementation guidance: +## Stage 6: Documentation And Examples -- Add an explicit app-layer mechanism to suppress per-report notification when - `GenerateReport` is called from `RunBatchDetailed`. -- Prefer a private field on `ReportRequest` or a private generation helper over - changing public CLI behavior. -- Ensure `GenerateReport` used by `generate ` still notifies exactly as - it does today. -- Suppress per-report notification for all `run morning` and `run evening` - report generation, regardless of whether batch notification is enabled, - disabled, skipped, or fails later. -- Ensure report metadata and report artifacts remain saved normally when - per-report notification is suppressed. -- Do not remove the existing per-report notification implementation because - single-report generation still uses it. +Goal: move implemented behavior from roadmap-only docs into maintained user and +internal docs. -Tests: +Documentation changes: -- `generate tomorrow` with distributor enabled calls the notifier once. -- `run evening` with distributor enabled does not call the notifier once per - generated report before the batch notification stage is wired in. -- Per-report notification artifacts are not written for batch-generated reports - when batch notification is enabled. -- Existing single-report notification artifact tests still pass. +- 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/`. -Run: +Tests and checks: -```sh -go test ./internal/app ./internal/cli -``` - -## Stage 6: Batch Notification Orchestration - -Goal: upload one distributor bundle after a fully successful batch and persist a -batch notification debug artifact. - -Implementation guidance: - -- In `RunBatchDetailed`, keep current report generation behavior: - - collect once; - - plan reports; - - continue generating later reports after individual failures; - - record per-report success/failure. -- After report generation finishes: - - if distributor is disabled, leave `BatchResult.Notification` nil; - - if distributor is enabled but `notify.distributor.batch.enabled=false`, set - batch notification status to disabled or omit the notification object, and - do not call distributor for the batch; - - if any report failed, set `BatchResult.Notification` to status `skipped` - with reason `one or more reports failed`, and do not call distributor; - - if all reports succeeded, build and send one batch notification request. -- Use the existing distributor adapter with multiple files. -- Save the batch notification artifact for attempted, succeeded, failed, and - status-error outcomes. -- Persist distributor accepted run ID, upload status, terminal run status, raw - run report JSON, status lookup error, and redacted failure error. -- Batch notification failure should: - - set top-level batch notification status to `failed`; - - increment or otherwise reflect aggregate batch failure consistently with - existing `BatchError` behavior; - - not mark individual report generation items as failed; - - return `BatchError` from `RunBatch` and an error from CLI execution. - -Tests: - -- All-success morning batch calls notifier exactly once with all report files. -- All-success evening batch calls notifier exactly once with all report files. -- Report failure skips notification and sets top-level notification status - `skipped`. -- Batch notification upload failure returns aggregate batch error and keeps - report item statuses succeeded. -- Batch notification run-status failure records raw status report JSON in the - debug artifact. -- Disabled distributor produces no notification object and no notifier call. -- Batch notification disabled produces no batch notifier call and no per-report - notifier calls from run commands. - -Run: - -```sh -go test ./internal/app ./internal/state ./internal/adapters/distributor -``` - -## Stage 7: CLI JSON And Stderr Output - -Goal: expose one batch notification result clearly without repeating it on every -report item. - -Implementation guidance: - -- Extend CLI JSON output naturally through `BatchResult.Notification`. -- Update stderr batch logging: - - keep existing compact per-report lines; - - add one `batchNotification ...` line when notification is attempted, - skipped, or failed; - - do not repeat the same batch notification error on every report line. -- Keep command syntax unchanged. -- Do not add distributor-specific CLI flags. -- Ensure secret values and bearer tokens cannot appear in JSON or stderr. - -Tests: - -- Batch JSON includes top-level notification fields for success. -- Batch JSON includes top-level skipped notification when a report failed. -- Batch JSON omits notification when distributor is disabled. -- Stderr includes one batch notification line on success/failure/skipped. -- Stderr does not repeat batch notification errors per report. -- Existing help output remains unchanged. - -Run: - -```sh -go test ./internal/cli ./internal/app -go run ./cmd/weatherreporter --help -``` - -## Stage 8: Documentation And Examples - -Goal: move implemented behavior out of roadmap-only status after the code is in -place. - -Implementation guidance: - -- Update `docs/config.md` with: - - `notify.distributor.batch.enabled`; - - batch pipeline, bundle ID, and idempotency templates; - - supported batch template variables; - - relationship between report path templates and batch bundle file mappings. -- Update `examples/config.yml` with the batch block and no secrets. -- Update `docs/operations.md` with: - - batch upload ordering; - - skip-all notification policy; - - batch artifact path; - - top-level batch notification JSON behavior. -- Update `docs/troubleshooting.md` with: - - skipped batch upload; - - batch upload failure; - - duplicate bundle path validation; - - distributor source conflicts. -- Update `docs/internal/app-orchestration.md` with batch notification workflow. -- Update `docs/internal/distributor-adapter.md` to clarify that multi-file - upload is supported by the adapter and batch orchestration lives in app. -- Update `docs/internal/state.md` with batch notification artifact paths and - JSON shape. -- Keep future extensions only under `docs/roadmap/`. - -Tests/checks: - -- Config examples load. -- Non-roadmap docs describe only implemented behavior. - -Run: - -```sh -go test ./internal/config -git diff --check -``` - -## Stage 9: Final Validation - -Run the full validation set: - -```sh -go test ./internal/app ./internal/config ./internal/state ./internal/cli ./internal/adapters/distributor +```bash +go test ./internal/config ./internal/app ./internal/adapters/distributor go test ./... go run ./cmd/weatherreporter --help git diff --check ``` -Manual checks: +Completion criteria: -- `weatherreporter generate tomorrow` still uses per-report notification. -- `weatherreporter run morning` sends at most one distributor upload. -- `weatherreporter run evening` sends at most one distributor upload. -- A failed report in a batch skips the batch upload. -- Batch upload source paths are managed report paths, not `--out-dir` copies. -- Batch upload bundle paths are unique. -- No token values appear in errors, batch JSON, notification artifacts, docs, or - examples. +- Maintained docs and examples match implemented behavior. +- Example config still loads in the config test suite. -## Deferred Work +## Final Verification -- Batch-level durable resume or retry queues. -- Batch-level distributor status inspection command. -- Multiple distributor pipelines within one batch. -- Report-type-specific distributor routing inside batch uploads. -- Uploading data packages, metadata, render contexts, or notification artifacts - in the batch bundle. -- Distributor-specific CLI flags. -- Batch manifest or progress system. -- Changing distributor destination merge semantics from weatherreporter. +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 roadmap decisions are sufficient for implementation: - -- failed report means skip all batch upload; -- initial batch upload uses one pipeline; -- batch debug artifacts live under `notifications/batches/...`; -- batch JSON uses one top-level `notification` object; -- single-report generation keeps per-report notification. +None. The prior roadmap questions are resolved above so the implementation can +proceed without additional product decisions.