Create a staged roadmap to address the issues identified in the code quality audit

2026-06-15 07:16:43 -05:00
parent ace5577402
commit ff2f8c16a3
1 changed files with 618 additions and 0 deletions
--- a/docs/roadmap/cleanup.md
+++ b/docs/roadmap/cleanup.md
@@ -0,0 +1,618 @@
+# 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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+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:
+
+```sh
+rg -n "validateGeneratedText|buildRenderContext|reportIDForCommand|reportBatchForCommand|reportIDForConfigKey" internal docs examples
+rg -n "ReportModuleOverrides" internal docs examples
+```
+
+Validation commands:
+
+```sh
+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:
+
+```sh
+go test ./...
+go run ./cmd/weatherreporter --help
+git diff --check
+```
+
+Additional checks after the full cleanup sequence:
+
+```sh
+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.