Create a roadmap and implementation plan for batch distributor uploads
This commit is contained in:
463
docs/roadmap/implementation.md
Normal file
463
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,463 @@
|
||||
# Batch Distributor Upload Implementation Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
Implement the batch distributor upload behavior defined in
|
||||
`docs/roadmap/batch-distributor.md`.
|
||||
|
||||
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.
|
||||
|
||||
## Source Roadmap
|
||||
|
||||
Use `docs/roadmap/batch-distributor.md` as the authoritative source for:
|
||||
|
||||
- user intent;
|
||||
- locked policy decisions;
|
||||
- target batch JSON shape;
|
||||
- batch notification artifact layout;
|
||||
- configuration shape;
|
||||
- app, state, and adapter boundaries;
|
||||
- deferred work.
|
||||
|
||||
Do not move aspirational batch distributor behavior into non-roadmap docs until
|
||||
the corresponding code is implemented.
|
||||
|
||||
## Locked Decisions
|
||||
|
||||
- `generate <report>` 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.
|
||||
|
||||
## Stage 1: Batch Notify Config And Template Rendering
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/config
|
||||
```
|
||||
|
||||
## Stage 2: Batch Notification State Artifacts
|
||||
|
||||
Goal: add batch-level notification artifact paths and persistence.
|
||||
|
||||
Implementation guidance:
|
||||
|
||||
- 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/<batch>/<local-date>/<batch_run_id>.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.
|
||||
|
||||
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.
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/state
|
||||
```
|
||||
|
||||
## Stage 3: Batch Notification App Types And Identity
|
||||
|
||||
Goal: add app-owned batch notification request/result types and deterministic
|
||||
batch run ID helpers.
|
||||
|
||||
Implementation guidance:
|
||||
|
||||
- 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
|
||||
<started_at_utc_timestamp>_<batch>
|
||||
```
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/app
|
||||
```
|
||||
|
||||
## Stage 4: Batch Bundle Request Construction
|
||||
|
||||
Goal: build a validated multi-file distributor notification request from a
|
||||
successful batch result.
|
||||
|
||||
Implementation guidance:
|
||||
|
||||
- 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.
|
||||
|
||||
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.
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/app ./internal/config
|
||||
```
|
||||
|
||||
## Stage 5: Suppress Per-Report Batch Notification
|
||||
|
||||
Goal: make run commands stop notifying once per report while preserving
|
||||
single-report notification.
|
||||
|
||||
Implementation guidance:
|
||||
|
||||
- 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 <report>` 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.
|
||||
|
||||
Tests:
|
||||
|
||||
- `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.
|
||||
|
||||
Run:
|
||||
|
||||
```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
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Manual checks:
|
||||
|
||||
- `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.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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.
|
||||
Reference in New Issue
Block a user