289 lines
9.8 KiB
Markdown
289 lines
9.8 KiB
Markdown
# 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.
|