14 KiB
Batch Collection Implementation Roadmap
Purpose
This is the staged implementation plan for docs/roadmap/batch.md. Implement
the stages in order. The feature has two goals:
- route all upstream Weather API collection through a canonical
internal/collectpackage; - update
run morningandrun eveningso they use one collected snapshot and dynamically add future Daily reports only for dates with complete hourly coverage.
This is an implementation roadmap only. Background, user intent, and target
policy live in docs/roadmap/batch.md.
Global Constraints
- Preserve public CLI syntax.
- Preserve managed workspace artifact paths.
- Preserve distributor upload behavior and paths.
- Keep Weather API HTTP details inside
internal/adapters/weatherapi. - Keep source collection in
internal/collect. - Keep batch planning in
internal/app. - Keep report definitions in
internal/report. - Do not make
internal/collectaware of report IDs, prompt IDs, or batch names. - Do not add config fields for partial Daily coverage.
- Do not create a workflow engine, scheduler package, or plugin system.
- Do not remove 3-Day or Weekend report definitions in this change.
Stage 1: Add Canonical Collection Package
Goal: introduce internal/collect as the only app-facing upstream collection
API while preserving current behavior.
Implementation:
-
Add
internal/collect. -
Define:
type Request struct { Config config.Config } type Result struct { Bundle *weatherdata.Bundle } -
Add
Run(ctx context.Context, req Request) (*Result, error). -
Runshould construct the Weather API adapter withweatherapi.Newand callclient.FetchBundle(ctx). -
Return an actionable error when adapter construction or fetch fails.
-
Keep the returned data normalized as
*weatherdata.Bundle; do not derivefacts.CollectedFactsorfacts.DerivedFactsin this package. -
Update
internal/app.FetchBundleandFetchAndSaveBundle, if retained, to callcollect.Runrather than constructingweatherapi.Newdirectly.
Tests:
- Add
internal/collecttests using a local Weather API test server or a small adapter seam if needed. - Keep existing
FetchAndSaveBundleapp tests passing. - Ensure collection errors include collection/fetch context without leaking secrets.
Acceptance:
internal/collectowns app-facing Weather API collection.internal/app.FetchBundleno longer constructsweatherapi.Newdirectly.- No report or batch behavior changes yet.
Validation:
go test ./internal/collect ./internal/app ./internal/adapters/weatherapi
git diff --check
Stage 2: Add App Collection Seam
Goal: make collection testable from app orchestration without requiring live Weather API calls.
Implementation:
-
Add a narrow app-owned collector interface, for example:
type Collector interface { Run(context.Context, collect.Request) (*collect.Result, error) } -
Add a default adapter that calls
collect.Run. -
Add
Collectorfields toGenerateRequestandBatchRequest. -
Default to the real collector when the request does not provide one.
-
Do not expose collector options through CLI.
Tests:
- Add app tests proving
Generateuses the provided collector. - Add app tests proving collection failure stops generation before report execution.
- Add batch tests proving
RunBatchDetailedcan use a fake collector in later stages.
Acceptance:
- App orchestration can be tested with a fake collector.
- CLI behavior is unchanged.
- External adapter types do not leak into app request or result structs.
Validation:
go test ./internal/app ./internal/cli
git diff --check
Stage 3: Make Report Generation Require Collected Data
Goal: remove hidden Weather API fetching from GenerateReport.
Implementation:
-
Add a collection field to
ReportRequest, for example:Collection collect.Result -
Update
Generateto:- collect once;
- resolve the requested report;
- pass the collection into
GenerateReport.
-
Update
GenerateReportto usereq.Collection.Bundle. -
If
req.Collection.Bundleis nil, return an actionable error such ascollected weather bundle is required. -
Remove the direct
FetchBundlecall fromGenerateReport. -
Keep
BuildReportFactsunchanged: it should still receive a*weatherdata.Bundle. -
Update all app tests that call
GenerateReportdirectly to pass a collected test bundle throughReportRequest.Collection.
Tests:
- Add or update tests proving single-report generation collects once.
- Add or update tests proving
GenerateReportfails before state writes when no collected bundle is provided. - Keep generated-text, notification, state, and output-copy tests passing with the explicit collection field.
Acceptance:
GenerateReporthas no direct Weather API fetch path.- Single-report commands still produce the same artifacts.
GenerateReporthas an explicit data dependency.
Validation:
go test ./internal/app ./internal/cli
rg -n "weatherapi\\.New|\\.FetchBundle\\(" internal
git diff --check
Expected grep matches at this stage should be limited to
internal/collect, internal/adapters/weatherapi, and their tests, plus any
transitional app fetch helper tests that explicitly verify FetchBundle.
Stage 4: Add Hourly Full-Day Coverage Helper
Goal: isolate the Daily eligibility rule before changing batch behavior.
Implementation:
- Add helper logic in
internal/app/batch_plan.go. - The helper should inspect
weatherdata.ForecastRunhourly periods and return eligible future Daily dates. - Inputs should include:
- hourly forecast run;
now;- loaded report timezone/location.
- The helper should:
- start candidate Daily expansion at the day after tomorrow;
- compute local civil days with
timeutil.CivilDay; - build required hourly start instants by stepping from civil-day start to civil-day end in one-hour increments;
- require every required start instant to exist as a valid hourly period
StartTime; - require matching periods to have valid end times after start times;
- scan candidate dates through the maximum local date represented by hourly period start times;
- return dates in ascending local date order;
- return no dates when hourly data is missing.
- Use exact start-time matching, not overlap-only matching.
Tests:
- Full ordinary local day with starts
00:00through23:00is eligible. - Missing one required hour makes the date ineligible.
- Partial final day is skipped.
- Today and tomorrow are never returned by expansion.
- Multiple eligible future days are returned in order.
- Non-hourly or invalid periods are ignored.
- DST transition days use actual civil-day hourly instants.
- Missing hourly forecast returns no dynamic Daily dates.
Acceptance:
- Daily eligibility is tested independently from batch execution.
- The helper does not resolve reports, call Weather API, write state, or invoke Scriptorium.
Validation:
go test ./internal/app
git diff --check
Stage 5: Add Shared Batch Planner
Goal: make morning and evening batch membership app-owned and data-aware.
Implementation:
-
Continue using
report.BatchForCommandNamefor CLI validation. -
Add an internal planned report type in
internal/app, for example:type plannedBatchReport struct { Resolved report.Resolved OutputCopyName string } -
Implement app-owned batch planning in
internal/app/batch_plan.go. -
Morning plan:
- resolve
report.Today; - resolve
report.Tomorrow; - resolve
report.Dailyfor each eligible future date from Stage 4.
- resolve
-
Evening plan:
- resolve
report.Tomorrow; - resolve
report.Dailyfor each eligible future date from Stage 4.
- resolve
-
Resolve dynamic Daily reports by setting
ResolveRequest.Dateto the target local date. -
Dynamic Daily planned reports should set
OutputCopyNametodaily-YYYY-MM-DD.md. -
Today and Tomorrow should use their report definition batch output names.
-
Remove
three-dayandweekendfrom morning planning. -
Stop using
report.Registry.BatchReportsfrom app batch execution. Either remove that method if it becomes unused, or leave it unused only if tests or docs still need it temporarily. Do not allow app batch execution to use it.
Tests:
- Morning order is Today, Tomorrow, then Daily dates.
- Evening order is Tomorrow, then Daily dates.
- Dynamic Daily dates start day after tomorrow.
- 3-Day and Weekend are absent from morning.
- Dynamic Daily resolves valid periods for the requested dates.
- Planned dynamic Daily output copy names are date-qualified.
- Unknown batch names still fail through existing CLI/report validation.
Acceptance:
- Batch membership is owned by
internal/app. - Batch membership is based on one collected bundle.
internal/collecthas no report or batch policy.
Validation:
go test ./internal/app ./internal/report ./internal/cli
git diff --check
Stage 6: Use One Collection Result Per Batch
Goal: make both batch commands collect once and reuse that collection for every report.
Implementation:
- Update
RunBatchDetailedto:- resolve the effective time;
- collect once using the request collector/default collector;
- plan the batch from that collection;
- pass the same
collect.Resultinto everyGenerateReportcall.
- If collection fails, return before generating any report.
- Update batch report result construction to use
plannedBatchReport. - Update
batchOutputPathor replace it with planned output-copy path logic:- if
OutputDiris empty, return empty output path; - if
planned.OutputCopyNameis non-empty, use that; - otherwise use
resolved.Definition.BatchOutputName; - join with
OutputDir.
- if
- Preserve per-report failure behavior after planning: later reports continue after a report failure.
- Preserve notification failure behavior.
Tests:
- Counted or fake collector proves one collection call for morning.
- Counted or fake collector proves one collection call for evening.
- Collection failure prevents any renderer calls.
- Report failure still allows later planned reports to run.
- Notification failure still marks only that report failed and continues.
--out-diroutput paths use:today.mdfor Today;tomorrow.mdfor Tomorrow;daily-YYYY-MM-DD.mdfor dynamic Daily.
Acceptance:
- Both batch commands use one collection result per invocation.
- No report in a batch performs its own upstream fetch.
- Batch JSON and stderr behavior remain coherent.
Validation:
go test ./internal/app ./internal/cli ./internal/state
rg -n "weatherapi\\.New|\\.FetchBundle\\(" internal
git diff --check
Expected grep matches should be limited to internal/collect,
internal/adapters/weatherapi, and their tests.
Stage 7: Remove Or Retire Legacy Static Batch Resolution
Goal: prevent future code from accidentally using stale batch membership.
Implementation:
- Inspect usages of
report.Registry.BatchReports. - If no longer needed, delete
Registry.BatchReportsand related tests. - If keeping a reduced helper is necessary, document in code comments that app batch planning is authoritative and ensure no production path calls the old static membership helper.
- Update report-registry tests that currently assert old morning/evening membership.
Tests:
- No production code path calls static registry batch membership.
- Report registry tests continue to cover report definitions, generated flags, batch output names, and valid-period resolution as appropriate.
Acceptance:
- There is no stale static morning/evening membership path in production code.
- Future agents cannot accidentally reintroduce old Today/3-Day/Weekend morning membership by calling a legacy helper.
Validation:
go test ./internal/report ./internal/app
rg -n "BatchReports\\(" internal
git diff --check
Expected BatchReports grep result should be empty or limited to tests or
comments that explicitly document it as non-production.
Stage 8: Documentation Updates
Goal: update implemented documentation after the code behavior changes.
Implementation:
- Update
docs/cli.md:- morning batch now generates Today, Tomorrow, and eligible future Daily reports;
- evening batch now generates Tomorrow and eligible future Daily reports;
- Daily eligibility is full hourly coverage.
- Update
docs/operations.mdwith the same operator-facing behavior and collection-failure behavior. - Update
docs/internal/app-orchestration.md:internal/collectboundary;- one collection per command;
- app-owned batch planning;
- explicit collected data passed to report generation.
- Update
docs/internal/report-registry.md:- report definitions remain canonical for report metadata;
- batch membership is app-owned when data-dependent.
- Add a new implemented internal component doc for
internal/collect, for exampledocs/internal/collect.md. - Do not describe future source families as implemented behavior.
Tests/checks:
- Grep docs for stale old batch language: Today/3-Day/Weekend morning, Weekend except Sunday, Daily not part of scheduled batches.
- Confirm docs do not imply
internal/collectowns report or batch policy.
Acceptance:
- Non-roadmap docs describe only implemented behavior.
- Operator docs and internal docs agree about batch membership.
Validation:
rg -n "3-Day|Weekend|scheduled batch|not part of scheduled|except on Sunday" docs README.md
git diff --check
Stage 9: Final Validation
Goal: verify the completed migration and behavior end to end.
Run:
go test ./...
go run ./cmd/weatherreporter --help
rg -n "weatherapi\\.New|\\.FetchBundle\\(" internal
rg -n "BatchReports\\(" internal
git diff --check
Manual review:
weatherapi.Newand.FetchBundle(matches are limited tointernal/collect,internal/adapters/weatherapi, and their tests.BatchReports(matches are absent from production paths, or explicitly documented as non-production if retained.run morningoutput summary can include multiple Daily reports without duplicate--out-dircopy paths.run eveninguses the same collection and dynamic Daily expansion logic as morning.- Managed workspace artifact paths are unchanged.
- Distributor upload paths are unchanged.
Open Questions
None.