weatherreporter/docs/roadmap/cleanup.md

Cleanup Roadmap

Purpose

This roadmap defines the staged cleanup work recommended by docs/roadmap/audit.md. It is written for an LLM coding agent that will implement the stages in order.

The cleanup is intended to simplify, centralize, and clarify implementation logic before the next major release without changing public CLI syntax, managed workspace paths, report IDs, prompt IDs, generated output semantics, or external integration behavior unless a stage explicitly says otherwise.

This file belongs under docs/roadmap/ because it describes planned refactoring work. Non-roadmap documentation must not describe the planned behavior here until the corresponding code is implemented.

Cleanup Principles

• Preserve implemented behavior unless a stage explicitly directs a change.
• Prefer narrow explicit helpers and registries over broad frameworks.
• Keep package boundaries from docs/policy/architecture.md:
  • internal/config owns config loading, defaults, overrides, secrets, and validation.
  • internal/report owns report identity, report definitions, valid periods, batches, output names, and comparison declarations.
  • internal/cli owns command parsing and help text.
  • internal/app owns orchestration.
  • internal/briefing owns prompt-facing module builders and module registry.
  • internal/generatedtext owns generated-text validation and render-context construction.
  • internal/reporttemplate owns embedded templates and generated-text schema assets.
  • external system types stay inside internal/adapters/*.
• Do not introduce Cobra, a workflow engine, plugin architecture, per-module packages, per-report packages, a manifest/resume system, or a global test framework.
• Keep tests focused near the package that owns the behavior.
• Run gofmt -w on changed Go files.
• Update implemented docs only in the stage that changes implemented behavior or package contracts.

Decisions Locked

• Generated-text/template resolution should use one explicit catalog rather than app-owned switches plus separate template/schema maps.
• Report definitions remain authoritative for report IDs, prompt IDs, generation mode, template ID, generated-text schema ID, valid-period resolver, module composition, artifact group, and output naming.
• internal/report should become the canonical home for report public-name and alias policy used by CLI, config, and app request resolution.
• App orchestration should keep a direct, readable workflow. Extract only narrow helpers for repeated policies such as final report completion.
• Config report-module normalization should be a load-time mutation. Validation should not perform duplicate mutating normalization.
• ReportModuleOverrides should not silently ignore invalid report keys in any path an application caller can reach.
• Weather API source lifecycle cleanup should be explicit and source-aware. Do not use reflection or a generic ingestion framework.
• Test cleanup should use package-local helpers only.
• State remains artifact-oriented. Do not add a manifest, checkpoint, resume, or run-index subsystem in this cleanup sequence.

Stage 1: Generated-Text And Template Catalog

Goal: make generated-text report asset resolution one explicit catalog so validators, schemas, templates, and render-context builders cannot drift independently.

Scope

Create a narrow catalog for generated-text-template reports. The catalog should connect:

• generated-text schema ID;
• embedded JSON schema asset;
• generated-text validator;
• report template ID;
• embedded Markdown template asset;
• render-context builder.

Implementation Guidance

• Prefer placing the primary catalog in internal/generatedtext if it owns validator and render-context function wiring.
• Keep embedded asset reading in internal/reporttemplate; do not move embedded template or schema files.
• Add an exported or package-internal lookup API with a small app-facing surface, for example:
  • generatedtext.LookupDefinition(def report.Definition);
  • or generatedtext.Lookup(schemaID, templateID string).
• The returned handler should expose methods or fields for:
  • Validate(data []byte) (any, []byte, error);
  • BuildRenderContext(metadata briefing.Metadata, snapshot module.Snapshot, facts app/report facts input, generated any) (any, error);
  • schema/template IDs needed by internal/reporttemplate.
• Avoid importing internal/app into internal/generatedtext. If app-owned ReportFacts is currently needed, pass its constituent facts.CollectedFacts and facts.DerivedFacts.
• Remove validateGeneratedText and buildRenderContext switches from internal/app after the catalog owns this dispatch.
• Keep internal/reporttemplate.Template, Schema, and Render as small asset helpers unless the catalog cleanly replaces their maps.
• If reporttemplate keeps template/schema maps, add tests tying those maps to the generated-text catalog. Prefer one source of truth if this is straightforward.

Files To Inspect

• internal/app/app.go
• internal/generatedtext/hourly.go
• internal/generatedtext/tomorrow.go
• internal/generatedtext/render_context.go
• internal/reporttemplate/reporttemplate.go
• internal/report/*_report.go
• internal/report/definition.go
• internal/reporttemplate/templates/*.md.tmpl
• internal/reporttemplate/schemas/*.schema.json

Acceptance Criteria

• Adding a new generated-text-template report requires registering one generated-text catalog entry and adding the report definition/assets, not editing app-level switches.
• internal/app no longer switches on concrete generated-text schema IDs or template IDs.
• Unsupported schema/template combinations return actionable errors that include the report ID and the unsupported ID.
• Existing hourly and tomorrow report behavior is unchanged.
• No generated-text schema, template, prompt ID, or report ID changes are introduced.

Tests

Add or update:

• Generated-text catalog completeness test proving every report with GenerationGeneratedTextTemplate has:
  • a generated-text catalog entry;
  • an embedded schema asset;
  • an embedded template asset;
  • a validator;
  • a render-context builder.
• Negative test for unsupported generated-text schema/template IDs.
• Existing hourly and tomorrow validation tests.
• Existing reporttemplate render tests.
• App workflow tests for hourly and tomorrow reports.

Validation commands:

go test ./internal/generatedtext ./internal/reporttemplate ./internal/report ./internal/app
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 2: Final Report Finalization Helper

Goal: centralize repeated metadata, output copy, notification, and result-population logic after a managed Markdown report exists.

Scope

Extract a narrow app helper that runs after either generation mode has produced the managed Markdown report file. This helper should not own Weather API fetching, facts, modules, Scriptorium calls, generated-text validation, or template rendering.

Implementation Guidance

• Add an unexported helper in internal/app, for example finalizeRenderedReport.
• Inputs should include the values already available in generation flow:
  • context;
  • loaded config;
  • store;
  • resolved report;
  • RunID;
  • metadata;
  • managed report path;
  • optional output path/output directory request;
  • report result being assembled, or enough fields to return one.
• The helper should centralize:
  • optional --out copy;
  • optional --out-dir copy;
  • final metadata updates related to report paths/output copy/notification;
  • final metadata save;
  • distributor notification invocation when enabled;
  • notification debug artifact save behavior already implemented;
  • notification failure propagation.
• Preserve current ordering:
  • managed report is written before finalization;
  • metadata is saved after final report path is known;
  • notification happens only after successful report generation and final metadata save.
• Preserve current behavior that distributor uses the managed report path, never optional output copies.
• Keep notification request construction in app unless a later stage creates a smaller named helper for template values.

Files To Inspect

• internal/app/app.go
• internal/state/metadata.go
• internal/state/filesystem.go
• internal/adapters/distributor
• internal/fileutil
• app tests covering generated reports, output copies, and notification artifacts.

Acceptance Criteria

• Direct Markdown report generation and generated-text-template generation both use the same finalization helper.
• Managed report paths, optional output copy behavior, metadata JSON shape, notification artifacts, and batch behavior are unchanged.
• Single-report notification failure still returns an error.
• Batch generation still continues independent reports and returns aggregate failure when any report fails.
• The helper is small enough to read without becoming a workflow engine.

Tests

Add or update:

• App test proving direct Markdown and generated-text-template reports both save final metadata consistently.
• App test proving --out and --out-dir still copy from the managed report path.
• App test proving notification receives the managed report path.
• App test proving notification failure fails a single report.
• Batch test proving notification failure marks only that report failed and the batch exits nonzero.

Validation commands:

go test ./internal/app ./internal/state ./internal/adapters/distributor
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 3: Report Public Name And Alias Resolution

Goal: make internal/report the canonical source for public report names, config keys, aliases, and batch command membership.

Scope

Move report-name resolution policy out of scattered CLI/app/config helpers and into report-owned helpers.

Implementation Guidance

• Add report-owned helpers in internal/report, for example:
  • IDForCommandName(name string) (ID, error);
  • IDForConfigKey(key string) (ID, error);
  • BatchForCommandName(name string) ([]ID, error);
  • CommandNames() []string if helpful for CLI help tests.
• Preserve current accepted names and aliases:
  • CLI report names currently accepted by generate;
  • config aliases currently accepted by reports.*;
  • batch names currently accepted by run.
• Keep user-facing error messages concise and actionable. It is acceptable for exact wording to change if tests are updated and the error remains clear.
• Update internal/cli to use report-owned command-name resolution or to call app request helpers that use it.
• Update internal/app to remove private reportIDForCommand and reportBatchForCommand mappings if report-owned helpers can replace them.
• Update internal/config to remove private reportIDForConfigKey in favor of report-owned config-key resolution.
• Avoid creating new exported app request types solely for this cleanup.

Files To Inspect

• internal/report/definition.go
• internal/report/registry.go
• internal/report/period.go
• internal/report/*_report.go
• internal/cli/root.go
• internal/app/app.go
• internal/config/reports.go
• docs/cli.md
• docs/config.md

Acceptance Criteria

• Report identity, command name, config key, and batch membership policy are discoverable from internal/report.
• CLI/app/config no longer maintain independent report ID switch statements for the same names.
• All existing public command names and config aliases continue to work unless explicitly documented as removed in this stage. This cleanup should not remove aliases.
• Distributor path variables that use report ID or artifact group are unchanged.

Tests

Add or update:

• Report tests for command-name-to-ID mapping.
• Report tests for config-key-to-ID mapping and aliases.
• Report tests for batch names and report order.
• CLI parser tests for every supported generate report name.
• Config tests for report override aliases and unknown keys.
• App tests for run batch command mapping if not covered through CLI.

Validation commands:

go test ./internal/report ./internal/cli ./internal/config ./internal/app
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 4: Config Report Module Normalization

Goal: remove duplicate mutating normalization from config validation and make report module override extraction explicit and safe.

Scope

Separate load-time config mutation from validation. Ensure report module overrides cannot silently ignore invalid report keys in app-facing paths.

Implementation Guidance

• Keep configuration loading order:
  1. built-in defaults;
  2. config file;
  3. CLI overrides;
  4. report module normalization;
  5. secrets directory loading;
  6. validation.
• Load should continue returning a fully normalized and validated Config.
• Refactor Validate so it does not perform duplicate mutating normalization.
• Options:
  • Preferred: split normalizeReportModules into a mutating load-time normalizer and a non-mutating validator used by Validate.
  • Acceptable: make Validate require normalized config and document/test that public callers should use Load for full processing.
• Do not remove config.Validate unless all callers and tests can clearly use Load.
• Update ReportModuleOverrides so invalid report keys are not silently ignored in app paths.
  • Preferred: change it to ReportModuleOverrides() (map[report.ID][]module.ConfigItem, error) and update callers.
  • Acceptable: keep the signature only if Config gains an internal validated/normalized marker and the method clearly cannot be reached with invalid keys.
• Preserve module option strict YAML decoding and composition validation.
• Preserve current config file syntax.

Files To Inspect

• internal/config/load.go
• internal/config/validate.go
• internal/config/reports.go
• internal/config/config.go
• internal/config/config_test.go
• internal/app/app.go
• examples/config.yml
• docs/config.md

Acceptance Criteria

• Report module normalization happens once on the normal Load path.
• Validation no longer performs duplicate mutating normalization.
• App report registry construction handles report override errors explicitly.
• Unknown report keys in report module overrides cannot be silently skipped.
• Existing valid example config still loads.
• Existing CLI overrides still take precedence over config and defaults.

Tests

Add or update:

• Config tests for normal load with typed module options.
• Config tests for direct validation of duplicate aliases and unknown report keys.
• Config tests proving ReportModuleOverrides returns or surfaces an error for invalid keys if called on invalid config.
• App test proving report registry construction fails clearly on invalid overrides when supplied programmatically.
• Existing example config load test.

Validation commands:

go test ./internal/config ./internal/app ./internal/cli
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 5: Generated-Text Internal Helper Cleanup

Goal: reduce low-risk duplication inside internal/generatedtext after the catalog is in place.

Scope

Add small helpers for repeated snapshot stanza extraction and strict JSON validation mechanics while preserving report-specific context structs and validation rules.

Implementation Guidance

Snapshot lookup:

• Add an unexported snapshot lookup/cache type, for example moduleSnapshotLookup.
• It should wrap module.Snapshot and provide typed methods for repeated module stanzas.
• Keep report-specific context structs:
  • HourlyRenderContext;
  • TomorrowRenderContext;
  • future report contexts.
• Do not replace typed context structs with a generic map[string]any.
• Preserve nil pointer behavior for omitted optional modules.

Strict JSON helper:

• Add a small helper for:
  • single JSON object decode;
  • DisallowUnknownFields;
  • detection of trailing JSON tokens;
  • canonical re-marshal of validated output.
• Keep report-specific required-field checks in hourly.go, tomorrow.go, and future report validators.
• Keep generated-text schema files unchanged unless tests show they are out of sync with validator behavior.

Files To Inspect

• internal/generatedtext/render_context.go
• internal/generatedtext/hourly.go
• internal/generatedtext/tomorrow.go
• internal/generatedtext/*_test.go
• internal/module
• internal/briefing

Acceptance Criteria

• Repeated optional stanza extraction is reduced without changing render-context JSON shape.
• Strict JSON decode behavior remains the same for hourly and tomorrow generated text.
• Unknown fields, missing required fields, trailing JSON, and invalid JSON errors remain actionable.
• Existing templates render unchanged.

Tests

Add or update:

• Generated-text validation tests for unknown fields, trailing JSON, missing required fields, and canonical output.
• Render-context tests proving key module pointers are populated when present and nil when omitted.
• Render-context tests proving extraction errors name the module ID.

Validation commands:

go test ./internal/generatedtext ./internal/reporttemplate ./internal/app
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 6: Weather API Source Lifecycle Helper

Goal: reduce repeated Weather API source fetch boilerplate while keeping source-specific semantics explicit.

Scope

Introduce a small helper or source spec for shared source lifecycle behavior in internal/adapters/weatherapi.

Implementation Guidance

• Centralize only common lifecycle mechanics:
  • endpoint and query construction;
  • envelope fetch;
  • absent/malformed/null data handling according to source policy;
  • source metadata construction;
  • warning/error conversion;
  • source hash handling.
• Keep source-specific decode functions explicit.
• Preserve these source-specific behaviors:
  • hourly forecast remains required for normal report generation;
  • alerts data:null means checked successfully with no active alerts;
  • optional non-alert source data:null follows missing-source policy;
  • weather story uses its current endpoint/query behavior;
  • SPC convective outlooks use the current endpoint constant and overlap filtering downstream;
  • current source warning and provenance JSON shapes remain unchanged.
• Do not use reflection, generics-heavy helpers, or a broad source framework.
• If the helper makes the code less readable, stop and keep the explicit source functions.

Files To Inspect

• internal/adapters/weatherapi/client.go
• internal/adapters/weatherapi/*_test.go
• internal/weatherdata
• internal/forecast
• docs/integrations/weatherapi.md
• docs/internal/weather-data.md

Acceptance Criteria

• Source fetch functions are shorter but still readable and source-aware.
• Existing source warning behavior is unchanged.
• Existing source metadata fields and hashes are unchanged.
• No Weather API transport details leak outside internal/adapters/weatherapi.
• No prompt/data package schema changes are introduced.

Tests

Add or update:

• Fixture-server test proving all expected endpoints are still requested.
• Required hourly data:null failure test.
• Alerts data:null no-active-alerts test.
• Optional non-alert missing-source policy tests for error, warn, and none where currently covered.
• Weather story metadata test.
• SPC source test if current coverage depends on source count or warning count.

Validation commands:

go test ./internal/adapters/weatherapi ./internal/weatherdata ./internal/forecast
go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt, but it should be skipped if the implementing agent cannot keep the helper narrow and explicit.

Stage 7: Package-Local Test Helper Cleanup

Goal: reduce noisy repeated test setup without creating a cross-package test framework.

Scope

Add package-local helpers only where tests already repeat substantial setup.

Implementation Guidance

• In internal/app tests, consider helpers for:
  • temporary config/workspace construction;
  • fake renderer setup;
  • fake notifier setup;
  • Weather API test server setup;
  • artifact path/glob assertions.
• In internal/cli tests, consider a command invocation helper that captures stdout/stderr and exit status.
• In internal/adapters/weatherapi tests, keep fixture server helpers local to the adapter package.
• In internal/adapters/distributor tests, keep fake upload/status clients local to the adapter package.
• Do not create internal/testutil or a global test helper package in this stage.
• Do not change test behavior or remove meaningful assertions while reducing setup.

Files To Inspect

• internal/app/*_test.go
• internal/cli/*_test.go
• internal/adapters/weatherapi/*_test.go
• internal/adapters/distributor/*_test.go
• internal/generatedtext/*_test.go
• internal/reporttemplate/*_test.go

Acceptance Criteria

• Test setup duplication is reduced in packages that have obvious repetition.
• Tests remain readable without hiding important workflow details.
• No production code changes are required for this stage unless an existing test-only seam is missing and justified.
• No global test helper package is introduced.

Tests

This stage changes tests only unless a small test seam is needed. Run:

go test ./internal/app ./internal/cli ./internal/adapters/weatherapi ./internal/adapters/distributor
go test ./...
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Stage 8: Documentation And Dead-Code Sweep

Goal: align implemented documentation with cleanup changes and remove stale symbols left behind by prior stages.

Scope

Update docs only for implemented cleanup changes. Remove stale code, stale tests, and stale roadmap references that no longer describe future work.

Implementation Guidance

• Update non-roadmap docs only where package contracts or contributor workflow changed:
  • docs/policy/development.md if package responsibilities or validation commands changed;
  • docs/internal/app-orchestration.md if finalization ordering is documented;
  • docs/internal/report-registry.md if report name/catalog helpers are documented;
  • docs/internal/generated-text.md or equivalent if generated-text catalog behavior is documented;
  • docs/internal/weather-data.md if Weather API source lifecycle behavior changed.
• Do not document deferred or unimplemented cleanup outside docs/roadmap/.
• Keep docs/roadmap/future.md for deferred ideas only if they remain future work.
• Remove stale private helpers after all call sites are gone.
• Run stale-symbol searches for removed helpers and old dispatch names.

Files To Inspect

• docs/policy/development.md
• docs/internal/
• docs/config.md
• docs/cli.md
• docs/operations.md
• docs/roadmap/
• production code touched by earlier stages

Acceptance Criteria

• Non-roadmap docs describe implemented behavior only.
• No stale references remain to removed private app switches or old helper names.
• Roadmap docs do not duplicate implemented docs except to identify deferred future work.
• go test ./..., CLI help, and whitespace checks pass.

Suggested Stale-Symbol Checks

Adjust names to match the actual implementation:

rg -n "validateGeneratedText|buildRenderContext|reportIDForCommand|reportBatchForCommand|reportIDForConfigKey" internal docs examples
rg -n "ReportModuleOverrides" internal docs examples

Validation commands:

go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Prompt size: This stage is suitable for one implementation prompt.

Deferred Refactors

Do not include these in the cleanup sequence:

• Generic workflow engine.
• Plugin architecture.
• Cobra migration.
• Per-module or per-report Go packages.
• Manifest, checkpoint, resume, progress, or run-index system.
• Reflection-heavy Weather API ingestion framework.
• Global test helper package.
• Generic adapter error wrapper.
• Config-only report definitions.
• Broad state-storage redesign around workspace scanning.

These may be revisited only if concrete new requirements make the current explicit structure materially expensive.

Global Validation Checklist

Run after each implementation stage unless the stage documents a narrower test set:

go test ./...
go run ./cmd/weatherreporter --help
git diff --check

Additional checks after the full cleanup sequence:

go test ./internal/generatedtext ./internal/reporttemplate ./internal/app
go test ./internal/report ./internal/cli ./internal/config
go test ./internal/adapters/weatherapi ./internal/adapters/distributor ./internal/adapters/scriptorium
go test ./internal/briefing ./internal/module ./internal/facts ./internal/forecast ./internal/promptinput ./internal/changes ./internal/state
rg -n "validateGeneratedText|buildRenderContext|reportIDForCommand|reportBatchForCommand|reportIDForConfigKey" internal docs examples

Manual review items:

• Public CLI syntax remains stable.
• Managed workspace paths remain stable.
• Distributor bundle paths remain stable.
• Report IDs, prompt IDs, template IDs, and generated-text schema IDs remain stable unless a stage explicitly changed them.
• No secret values appear in errors, logs, metadata, notification artifacts, docs, examples, or tests.
• Non-roadmap documentation describes only implemented behavior.

Open Questions

No open questions block implementation of this cleanup roadmap.

The only discretionary item is Stage 6. Recommended approach: implement a narrow Weather API source lifecycle helper only if it remains explicit and improves readability. Viable alternative: skip Stage 6 and keep the current source-specific methods until the next Weather API source is added. The alternative is acceptable because current source functions are readable and tested, and the risk is mostly future maintenance cost rather than current behavior drift.