From 1e9c29aa55d53af53c6ea9440e5594828369a5a5 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 10 Jun 2026 07:49:29 -0500 Subject: [PATCH] Cleanup after implementation of the module architecture and remove completed roadmap files --- docs/internal/module.md | 10 +- docs/roadmap/distributor.md | 34 -- docs/roadmap/future.md | 85 ++- docs/roadmap/implementation.md | 791 --------------------------- docs/roadmap/modules.md | 677 ----------------------- examples/config.yml | 1 - internal/app/app.go | 7 - internal/briefing/modules.go | 103 ++-- internal/briefing/modules_test.go | 65 ++- internal/config/config_test.go | 12 +- internal/module/module.go | 31 +- internal/promptinput/package_test.go | 2 +- internal/report/period_test.go | 6 - internal/report/registry.go | 5 - 14 files changed, 222 insertions(+), 1607 deletions(-) delete mode 100644 docs/roadmap/distributor.md delete mode 100644 docs/roadmap/implementation.md delete mode 100644 docs/roadmap/modules.md diff --git a/docs/internal/module.md b/docs/internal/module.md index 79920a7..18ff5a8 100644 --- a/docs/internal/module.md +++ b/docs/internal/module.md @@ -33,21 +33,15 @@ The registry recognizes these IDs: - `current_conditions` - `derived_daily_summary` - `derived_daypart_summaries` -- `hourly_table` - `precip_timing` - `alert_digest` - `area_forecast_discussion` - `weather_story` -- `forecast_delta` - `outdoor_windows` - `tomorrow_planning` -- `weekend_planning` -- `storm_window_summary` -Modules with builders emit stanzas into module snapshots. Registered modules -without builders are valid composition entries but do not emit snapshot stanzas. -That keeps report composition declarations centralized while limiting prompt -packages to data the application builds. +Every registered module has a builder. Report composition entries that refer to +unknown or unimplemented module IDs fail validation instead of being skipped. ## Options diff --git a/docs/roadmap/distributor.md b/docs/roadmap/distributor.md deleted file mode 100644 index 43ccfdc..0000000 --- a/docs/roadmap/distributor.md +++ /dev/null @@ -1,34 +0,0 @@ -# Distributor Roadmap - -Current distributor notification behavior is documented outside the roadmap: - -- [Configuration reference](../config.md) -- [Operations guide](../operations.md) -- [Troubleshooting](../troubleshooting.md) -- [Distributor adapter internals](../internal/distributor-adapter.md) - -This file tracks future distributor-related work only. - -## Deferred Enhancements - -- Add a supported warning-only notification policy. -- Include selected non-report artifacts in uploaded bundles. -- Poll distributor run status after upload acceptance. -- Persist upload retry state across process restarts. -- Add explicit CLI controls for distributor behavior. - -## Non-Goals Without A Separate Design - -- Do not make distributor scan the weatherreporter workspace. -- Do not move destination routing into weatherreporter. -- Do not move Markdown-to-HTML transformation into weatherreporter. -- Do not store raw bearer tokens in configuration files. - -## Required Constraints For Future Work - -- Distributor package types stay inside `internal/adapters/distributor`. -- Weatherreporter submits explicit source bundles built from generated files. -- Optional `--out` and `--out-dir` copies remain operator conveniences, not - canonical upload sources. -- Secret values stay out of errors, logs, CLI output, metadata, examples, and - documentation. diff --git a/docs/roadmap/future.md b/docs/roadmap/future.md index cae00e2..0a78527 100644 --- a/docs/roadmap/future.md +++ b/docs/roadmap/future.md @@ -3,7 +3,7 @@ This roadmap contains project work that is not implemented. Current behavior is documented outside `docs/roadmap/`. -## Deferred: Automatic Storm Monitoring +## Automatic Storm Monitoring Manual Storm Report generation is implemented through `weatherreporter generate storm --start TIME --end TIME`. Automatic storm-event @@ -34,30 +34,56 @@ Acceptance criteria before implementation: - evaluator failures are inspectable and do not create noisy report output; - manual Storm Report generation remains available. -## Deferred: Alternate Runtime Integrations +## Future Report Types And Modules -These ideas are not current behavior: +The module-based prompt package architecture is implemented. Future work should +add only modules backed by implemented upstream facts and clear report needs. -- native LLM client inside `weatherreporter`; -- database-backed state; -- public HTTP API; -- multi-location selection; -- daemon mode; -- multi-user authorization; -- plugin system. +Possible future report types: -Each item needs its own design note before implementation. Non-roadmap docs -must not describe these as available behavior. +- `next_6_hours` or another short-fuse planning report; +- event-specific reports with stable event IDs; +- storm review or yesterday-style reports using historical observations; +- archive-focused report variants if generated report history becomes a + first-class product. -## Deferred: Distributor Notification Enhancements +Possible future modules: + +- `hourly_table` for compact valid-period hourly facts; +- `forecast_delta` if a separate stanza is useful beyond current Recent + Changes; +- `weekend_planning` if weekend-specific planning guidance needs a dedicated + deterministic stanza; +- `storm_window_summary` if manual or automatic Storm Reports need a dedicated + prompt-facing storm-window module; +- separate AFD section aliases, such as `afd_key_messages`, + `afd_short_term_text`, and `afd_long_term_text`, if separate stanzas prove + more useful than `area_forecast_discussion.options.sections`; +- SPC, radar, QPF, snow/rain total, or historical-observation modules once + upstream sources and report requirements exist. + +QPF fields such as `measurable_qpf_total_in` and `max_hourly_qpf_in` should +remain omitted until a real upstream quantitative precipitation source is +represented in `CollectedFacts`. + +Future module work should preserve these boundaries: + +- collect upstream facts once per report run; +- keep upstream fetching out of modules; +- keep broad reusable calculations in `DerivedFacts`; +- keep prompt-facing field shape inside module builders; +- use typed options for configurable module behavior; +- keep module snapshots structured and deterministic for Recent Changes. + +## Distributor Notification Enhancements Distributor notification currently uploads one managed Markdown report per -successful generated report through the configured HTTP upload endpoint. +successful generated report through the configured HTTP upload pipeline. These enhancements are not current behavior: - `failure_policy: warn`; -- uploading metadata, briefing snapshots, data packages, or preflight artifacts; +- uploading metadata, module snapshots, data packages, or preflight artifacts; - polling distributor status after upload acceptance; - durable upload retry queues; - distributor-specific CLI flags; @@ -69,7 +95,26 @@ Any distributor enhancement should preserve the existing adapter boundary: weatherreporter selects explicit generated files and submits source bundles, while distributor owns destination routing and publication behavior. -## Deferred: Cleanup Refactors +## Alternate Runtime Integrations + +These ideas are not current behavior: + +- native LLM client inside `weatherreporter`; +- database-backed state; +- public HTTP API; +- multi-location selection; +- daemon mode; +- multi-user authorization; +- plugin system; +- dynamic module loading; +- user-defined module code; +- YAML-defined module schemas; +- module-owned Weather API fetching. + +Each item needs its own design note before implementation. Non-roadmap docs +must not describe these as available behavior. + +## Cleanup Refactors The initial cleanup pass intentionally left these refactors out because the current implementation does not yet make them worth the added abstraction. @@ -81,14 +126,12 @@ more expensive: - Weather API optional-source specification/helper refactor: consider when additional Weather API sources make per-source fan-out, policy handling, and provenance wiring repetitive enough to obscure adapter behavior. -- Broad briefing weather-signal consolidation: consider when multiple briefing +- Broad briefing weather-signal consolidation: consider when multiple module builders repeatedly derive the same weather signals and tests begin to need coordinated fixture updates. - Generic workflow engine: defer unless generation, inspection, recovery, or future background workflows gain enough shared step semantics to justify a declared execution model. -- Plugin architecture: defer until there is a concrete external extension - contract and at least one implemented extension point. - Cobra migration: defer while the standard-library CLI remains small, explicit, and covered by parser tests. - Manifest, resume, or progress system: defer until operators need resumable @@ -101,5 +144,5 @@ more expensive: artifact output. Any future implementation should preserve the existing public CLI, artifact -paths, report identities, and adapter boundaries unless a separate roadmap -explicitly changes them. +paths, report identities, module boundaries, and adapter boundaries unless a +separate roadmap explicitly changes them. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index 896bea1..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,791 +0,0 @@ -# Modular Data Package Implementation Roadmap - -This roadmap is a staged implementation plan for -[`docs/roadmap/modules.md`](modules.md). It is future-work planning only. The -target audience is an LLM coding agent implementing each stage in order. - -## Purpose - -Implement a pre-release hard cutover from report-shaped briefing packages to a -module-oriented prompt package architecture: - -```text -CollectedFacts -> DerivedFacts -> ModuleOutput -``` - -The implementation should produce YAML `data_package` artifacts with named -stanzas for Scriptorium prompts, persist JSON module snapshots for inspection -and Recent Changes, and make report composition configurable through ordered -module IDs plus typed module options. - -## Source Roadmap - -[`docs/roadmap/modules.md`](modules.md) is authoritative for the conceptual -policy, user intent, target prompt shape, boundaries, and acceptance criteria -for this refactor. This document is authoritative for implementation order, -stage scope, file/package guidance, and validation commands. - -If this implementation plan appears to conflict with `modules.md`, stop and -reconcile the roadmap before changing code. Do not infer a different policy -from stage sequencing. - -## Locked Decisions - -- Do a clean break. Do not preserve old report-shaped briefing JSON as a - compatibility layer. -- Introduce `internal/weatherdata` for normalized collected source types. -- Keep `internal/forecast` for forecast-specific derivation algorithms such as - period selection, daily summaries, daypart grouping, and precipitation - timing. -- Introduce an internal fact contract for `CollectedFacts` and `DerivedFacts`. -- Introduce a narrow module contract for module IDs, typed options, outputs, - and snapshots. -- Persist both artifacts: - - JSON module snapshots for state, inspection, and Recent Changes; - - YAML prompt data packages passed to Scriptorium as `data_package`. -- Replace `weatherreporter inspect briefing` with - `weatherreporter inspect modules`. -- Support typed module options from the first configurable composition pass. -- Use named YAML stanzas, not a generic array of module objects. -- Omit QPF fields until a real upstream QPF source is represented in - `CollectedFacts`. -- Keep public generate/run command names, report IDs, prompt IDs, RunID format, - managed Markdown report paths, and distributor upload source stable. -- Do not introduce plugins, dynamic loading, generic workflow engines, or - module-owned upstream fetching. - -## Target Packages - -Implementation should converge on this package ownership: - -- `internal/weatherdata`: normalized collected source facts, source metadata, - source warnings, and broad weather data types. -- `internal/forecast`: deterministic forecast-specific algorithms over - `weatherdata` types. -- `internal/facts`: `CollectedFacts`, `DerivedFacts`, and their builders. -- `internal/module`: stable module IDs, module output envelope, module snapshot - shape, module config item shape, and shared option/output contracts that must - be imported by both `internal/report` and `internal/briefing`. -- `internal/briefing`: module registry and module builders. -- `internal/report`: report definitions, valid periods, output identity, - comparison strategy, and default module composition. -- `internal/config`: YAML config structs, defaults, loading, and validation for - module composition overrides. -- `internal/promptinput`: YAML prompt package assembly, validation, and save - behavior. -- `internal/changes`: structured comparison over module snapshots. -- `internal/state`: module snapshot paths, YAML data package paths, metadata - links, and inspection loads. -- `internal/app`: orchestration only. - -Avoid import cycles. In particular, `internal/report` may import -`internal/module` for module IDs, but `internal/module` must not import -`internal/report`. - -## Target Artifacts - -Use explicit schema versions: - -- Module snapshot JSON: `weatherreporter.modules.v1` -- YAML prompt data package: `weatherreporter.data_package.v2` - -Target workspace paths: - -```text -workspace/ - snapshots///.modules.json - snapshots///.metadata.json - data-packages///.data_package.yaml -``` - -Metadata should link the module snapshot and YAML data package paths. Existing -metadata links for preflight, rendered report, source warnings, source hashes, -and distributor notification artifacts should remain. - -## Target Module Defaults - -Initial implemented default module IDs should cover current behavior without -QPF-specific fields: - -- `metadata` -- `current_conditions` -- `derived_daily_summary` -- `derived_daypart_summaries` -- `precip_timing` -- `alert_digest` -- `area_forecast_discussion` -- `weather_story` -- `forecast_delta` -- `outdoor_windows` -- `weekend_planning` -- `storm_window_summary` - -Default report composition should be declared in `internal/report`: - -- Daily Today: - `metadata`, `current_conditions`, `derived_daily_summary`, - `derived_daypart_summaries`, `precip_timing`, `alert_digest`, - `forecast_delta`, `area_forecast_discussion`, `weather_story`, - `outdoor_windows` -- Daily Tomorrow: - same as Daily Today, plus any tomorrow-planning module needed to preserve - current tomorrow behavior. -- 3-Day: - `metadata`, `current_conditions`, `derived_daypart_summaries`, - `precip_timing`, `alert_digest`, `forecast_delta`, - `area_forecast_discussion`, `weather_story`, `outdoor_windows` -- Weekend: - `metadata`, `current_conditions`, `derived_daypart_summaries`, - `precip_timing`, `alert_digest`, `area_forecast_discussion`, - `weather_story`, `outdoor_windows`, `weekend_planning` -- Storm: - `metadata`, `current_conditions`, `hourly_table`, `precip_timing`, - `alert_digest`, `area_forecast_discussion`, `weather_story`, - `storm_window_summary` - -If preserving a current report behavior requires a narrower module, add a -specific module rather than keeping old report-shaped containers. - -## Stage 1: Weatherdata Package Split - -Goal: separate broad normalized weather data from forecast-specific derivation. - -Files/packages to change: - -- create `internal/weatherdata`; -- update `internal/forecast`; -- update `internal/adapters/weatherapi`; -- update packages that currently import normalized source types from - `internal/forecast`. - -Implementation guidance: - -- Move normalized source/domain types out of `internal/forecast` when they are - not forecast algorithms: - - bundle/source metadata/warnings; - - current conditions; - - observation run types if present; - - alert run and alert overlap source types; - - forecast run and forecast period source types; - - discussion and discussion section types; - - weather story types. -- Keep deterministic derivation functions in `internal/forecast`. -- Update Weather API adapter return types to use `weatherdata.Bundle`. -- Keep JSON field names and Weather API fixture behavior unchanged. -- Do not change CLI behavior, artifact paths, or prompt input yet. - -Acceptance criteria: - -- Weather API adapter tests pass with `weatherdata` types. -- Forecast derivation tests pass using `weatherdata` inputs. -- No external adapter dependency types leak into `weatherdata`. -- Existing generated report behavior is unchanged at this stage. - -Validation: - -```bash -go test ./internal/weatherdata ./internal/forecast ./internal/adapters/weatherapi -go test ./internal/app ./internal/briefing ./internal/promptinput -``` - -This stage is suitable for one implementation prompt if kept mechanical. - -## Stage 2: Fact Contracts - -Goal: add explicit `CollectedFacts` and `DerivedFacts` contracts. - -Files/packages to change: - -- create `internal/facts`; -- update `internal/app`; -- update `internal/forecast` tests as needed. - -Implementation guidance: - -- Define `CollectedFacts` as normalized upstream facts collected once per - report run. -- Define `DerivedFacts` as conservative, reusable, report-scoped - transformations. -- Add builders: - - `BuildCollected(bundle *weatherdata.Bundle) CollectedFacts` - - `BuildDerived(req BuildDerivedRequest) (DerivedFacts, error)` -- `BuildDerivedRequest` should include the resolved report, timezone, - configured dayparts, and `CollectedFacts`. -- `DerivedFacts` may include: - - valid-period hourly periods; - - valid-period narrative periods; - - alert overlaps; - - daily summaries; - - daypart summaries where reusable; - - precipitation timing if reused by multiple modules. -- Do not put prompt wording, prose strings, module-specific ranking, or - one-off presentation decisions in `DerivedFacts`. -- Keep source provenance and warnings separate from ordinary fact access. - -Acceptance criteria: - -- `CollectedFacts` can be built once from a fetched bundle. -- `DerivedFacts` can be built for Daily, Tomorrow, 3-Day, Weekend, and Storm. -- Derived fact builders have tests for valid-period slicing, daypart grouping, - alert overlaps, and missing optional sources. -- No module or prompt code exists yet that fetches upstream data. - -Validation: - -```bash -go test ./internal/facts ./internal/forecast ./internal/app -go test ./internal/... -``` - -This stage is suitable for one implementation prompt. - -## Stage 3: Module Core Contracts - -Goal: define module IDs, options, outputs, snapshots, and registry mechanics. - -Files/packages to change: - -- create `internal/module`; -- update `internal/report`; -- update `internal/briefing`. - -Implementation guidance: - -- Define: - - `module.ID`; - - module ID constants; - - `module.ConfigItem`; - - `module.Output`; - - `module.Snapshot`; - - shared schema version constants. -- `module.Output` should contain module ID, stanza name, and typed value. -- `module.Snapshot` should preserve ordered outputs and support typed stanza - lookup for comparison code. -- Add duplicate module and duplicate stanza-name validation. -- Add typed option structs for initial modules. Empty option structs are fine - for modules without options. -- Add a module registry in `internal/briefing` that maps module IDs to builder - definitions. -- Module definitions should declare: - - ID; - - stanza name; - - option type; - - default options; - - required collected facts; - - required derived facts; - - supported report IDs or report categories; - - missing-data behavior. -- Do not execute modules from app orchestration yet unless needed for tests. - -Acceptance criteria: - -- Report definitions can refer to `module.ID` without import cycles. -- Module registry tests reject unknown modules, duplicate module IDs, duplicate - stanza names, incompatible reports, and invalid option shapes. -- Module output and snapshot JSON marshal deterministically enough for tests. - -Validation: - -```bash -go test ./internal/module ./internal/briefing ./internal/report -``` - -This stage is suitable for one implementation prompt. - -## Stage 4: Base Modules - -Goal: implement source-oriented modules that mostly pass through normalized or -lightly selected facts. - -Files/packages to change: - -- `internal/briefing`; -- `internal/module`; -- tests under `internal/briefing`. - -Implementation guidance: - -- Implement these modules: - - `metadata`; - - `current_conditions`; - - `alert_digest`; - - `area_forecast_discussion`; - - `weather_story`. -- The `metadata` module should expose report metadata, configured location, - units, timezone, valid period, source warnings summary, and alert checked - status where appropriate. -- `area_forecast_discussion` should expose key messages, short-term text, and - long-term text when present. -- `weather_story` should expose structured story fields when present and omit - the stanza when missing/suppressed by missing-source policy. -- `alert_digest` should distinguish checked/no-active-alerts from missing alert - source data. -- Ordinary modules should not expose endpoint, hash, or transport provenance; - provenance should remain metadata/source-warning oriented. - -Acceptance criteria: - -- Each module has focused tests for available data, missing optional data, and - empty output omission. -- No module fetches upstream data or reads/writes durable state. -- Output field names use YAML-friendly snake_case and unit suffixes where - needed. - -Validation: - -```bash -go test ./internal/briefing ./internal/module ./internal/facts -``` - -This stage is suitable for one implementation prompt. - -## Stage 5: Derived Fact Modules - -Goal: implement deterministic modules that package reusable forecast -derivations for the LLM. - -Files/packages to change: - -- `internal/forecast`; -- `internal/facts`; -- `internal/briefing`; -- `internal/module`. - -Implementation guidance: - -- Implement: - - `derived_daily_summary`; - - `derived_daypart_summaries`; - - `precip_timing`; - - `outdoor_windows`; - - any tomorrow-planning module needed to preserve Tomorrow output quality. -- `derived_daily_summary` should include current implementable fields: - - `high_temp_f`; - - `low_temp_f`; - - `max_pop_percent`; - - `max_pop_window`; - - `first_precip_hour`; - - `last_precip_hour`; - - `thunder_mentioned`; - - `max_wind_gust_mph`; - - `heat_index_max_f` when source data supports it. -- Do not implement `measurable_qpf_total_in` or `max_hourly_qpf_in` until QPF - exists in `CollectedFacts`. -- `derived_daypart_summaries` should expose daypart keyed values using the - configured daypart definitions. -- Keep broad reusable calculations in `DerivedFacts`; keep prompt-shape - packaging inside modules. - -Acceptance criteria: - -- Derived modules have fixture coverage across ordinary, dry, rainy, windy, - cold/heat, and missing-data scenarios. -- QPF fields are absent unless an upstream QPF source exists. -- Daily and Tomorrow module outputs contain enough data to replace current - report-shaped daily briefing content. - -Validation: - -```bash -go test ./internal/forecast ./internal/facts ./internal/briefing ./internal/module -``` - -This stage may be too large for one prompt if all modules are implemented at -once. Split into Daily-derived modules first, then outlook/storm derived -modules if needed. - -## Stage 6: Report Composition And Config Overrides - -Goal: make report definitions and config the source of module composition. - -Files/packages to change: - -- `internal/report`; -- `internal/config`; -- `examples/config.yml`; -- config tests. - -Implementation guidance: - -- Extend `report.Definition` with default ordered module IDs. -- Keep valid-period resolution, prompt IDs, output naming, generated flag, and - comparison strategy in `internal/report`. -- Add config support: - -```yaml -reports: - daily: - deterministic_modules: - - current_conditions - - id: area_forecast_discussion - options: - sections: - - short_term -``` - -- Support both string shorthand and object form for module entries. -- Normalize config into typed `module.ConfigItem` values. -- Decode module options into typed option structs during validation or before - module execution. -- Reject: - - unknown report IDs; - - unknown module IDs; - - duplicate modules unless explicitly allowed by that module; - - duplicate stanza names; - - incompatible report/module combinations; - - invalid options. -- Built-in defaults should work when no report module config is present. -- Example config may omit module overrides unless an example is needed. - -Acceptance criteria: - -- Defaults reproduce intended module composition for all implemented reports. -- A config edit can add/remove an implemented module for a report. -- Invalid module config errors are actionable and do not mention raw internal - panic/details. -- Config examples load. - -Validation: - -```bash -go test ./internal/report ./internal/config ./internal/briefing -go run ./cmd/weatherreporter --help -``` - -This stage is suitable for one implementation prompt. - -## Stage 7: Module Snapshot State - -Goal: persist and inspect JSON module snapshots without changing Scriptorium -input yet. - -Files/packages to change: - -- `internal/state`; -- `internal/app`; -- `internal/cli`; -- app/state/CLI tests. - -Implementation guidance: - -- Add state paths for `.modules.json`. -- Add save/load methods for module snapshots. -- Update metadata to include `ModuleSnapshotPath`. -- Add `weatherreporter inspect modules [--config PATH] RUN_ID`. -- Remove `inspect briefing` from parser support and help text in this stage. -- Keep old data package generation in place only until Stage 8, but do not - leave generation without a module snapshot. - -Acceptance criteria: - -- Generated runs persist module snapshots before prompt package construction. -- `inspect modules` returns the module snapshot. -- `inspect briefing` is gone from help text and parser tests. -- Metadata links the module snapshot path. -- Existing report generation still succeeds with fake Scriptorium. - -Validation: - -```bash -go test ./internal/state ./internal/cli ./internal/app -go run ./cmd/weatherreporter --help -``` - -This stage is suitable for one implementation prompt. - -## Stage 8: YAML Prompt Package Cutover - -Goal: replace JSON prompt data packages with YAML named-stanza data packages. - -Files/packages to change: - -- `internal/promptinput`; -- `internal/state`; -- `internal/adapters/scriptorium` tests; -- `internal/app`. - -Implementation guidance: - -- Set prompt package schema version to `weatherreporter.data_package.v2`. -- Build prompt package content from module snapshots, report metadata, recent - changes, and source warnings. -- Save prompt packages as `.data_package.yaml`. -- Continue passing Scriptorium input as `--input data_package=`. -- Update render/run tests to avoid assuming `.json` filenames. -- Ensure YAML uses named stanzas under `briefing`. -- Omit empty optional fields. -- Keep module snapshot JSON as the comparison/inspection source. -- Update metadata `DataPackagePath` to point to YAML. - -Acceptance criteria: - -- Scriptorium render and run receive a YAML `data_package` path. -- YAML output is deterministic enough for tests. -- `inspect data-package` returns YAML content or a parsed representation - chosen consistently in CLI tests. -- No code assumes data package paths end in `.json`. - -Validation: - -```bash -go test ./internal/promptinput ./internal/adapters/scriptorium ./internal/state ./internal/app ./internal/cli -go test ./... -``` - -This stage is suitable for one implementation prompt. - -## Stage 9: App Orchestration Cutover - -Goal: make module execution the only generation path for all implemented -reports. - -Files/packages to change: - -- `internal/app`; -- `internal/briefing`; -- `internal/facts`; -- app workflow tests. - -Implementation guidance: - -- In `GenerateReport`, fetch Weather API data once, build `CollectedFacts`, - build `DerivedFacts`, execute configured modules, save module snapshot, build - YAML prompt package, then continue preflight/run/metadata/distributor flow. -- Preserve ordering: - 1. resolve prior comparable metadata; - 2. fetch bundle; - 3. build facts; - 4. execute modules; - 5. save module snapshot; - 6. compute Recent Changes; - 7. save YAML data package; - 8. run render preflight; - 9. save metadata; - 10. run Scriptorium; - 11. copy optional output; - 12. save final metadata; - 13. notify distributor if enabled. -- Do not use `--out` or `--out-dir` copies for distributor notification. -- Do not invoke modules after Scriptorium failures. -- Keep batch behavior unchanged: continue independent reports, return nonzero - aggregate status if any report fails. - -Acceptance criteria: - -- Daily, Tomorrow, 3-Day, Weekend, and Storm generation all use module - snapshots and YAML data packages. -- Existing public CLI syntax remains stable except `inspect modules` replacing - `inspect briefing`. -- Managed Markdown report paths and distributor upload source remain stable. -- App tests assert generated module snapshots and YAML data packages. - -Validation: - -```bash -go test ./internal/app ./internal/cli ./internal/state ./internal/briefing ./internal/promptinput -go test ./... -``` - -This stage may be large. Split by report family if needed: Daily/Tomorrow, -Outlooks, then Storm. - -## Stage 10: Recent Changes Migration - -Goal: compare structured module snapshots instead of report-shaped briefing -packages. - -Files/packages to change: - -- `internal/changes`; -- `internal/state`; -- `internal/app`; -- changes tests. - -Implementation guidance: - -- Define which module stanzas each comparison strategy consumes. -- Daily comparison should use `derived_daily_summary`, - `derived_daypart_summaries`, `alert_digest`, and `precip_timing` where - present. -- 3-Day and Weekend comparisons should use module snapshot outputs that replace - current outlook day comparisons. -- Storm comparison should remain explicit-window based and consume storm - module outputs when implemented. -- Do not compare rendered Markdown or rendered YAML text. -- If a comparison-required module is missing, return an actionable error or an - inspectable warning according to the report policy chosen in code. Prefer an - error for required comparison modules and no-op only for optional comparison - stanzas. - -Acceptance criteria: - -- Prior snapshot lookup still uses report compatibility and valid-period rules. -- Recent Changes output remains deterministic. -- Tests cover unchanged forecasts, threshold-crossing changes, alert changes, - precip timing changes, and missing comparison stanzas. -- Old `briefing.Package` comparison code is removed. - -Validation: - -```bash -go test ./internal/changes ./internal/state ./internal/app -go test ./... -``` - -This stage is suitable for one implementation prompt if module snapshots are -already available. - -## Stage 11: Remove Old Briefing Shapes - -Goal: remove obsolete report-shaped briefing containers and stale JSON package -assumptions. - -Files/packages to change: - -- `internal/briefing`; -- `internal/promptinput`; -- `internal/state`; -- `internal/app`; -- tests throughout `internal`. - -Implementation guidance: - -- Remove old `Daily`, `ThreeDay`, `Weekend`, and `Storm` briefing container - structs when no longer used. -- Remove old `briefing.Package` if it no longer represents the module - snapshot. If the package keeps a `Package` type, it must be module-oriented. -- Remove tests that construct old report-shaped briefing fixtures. -- Remove stale `.data_package.json` assumptions. -- Remove dead helper functions that only supported old report-shaped output. -- Keep generated report Markdown behavior stable. - -Acceptance criteria: - -- `rg -n "data_package\\.json|inspect briefing|briefing\\.Package" internal docs -g '!docs/roadmap/**'` - has no production-code matches, except deliberate roadmap/history references - where appropriate. -- No old report-shaped content structs remain on the generation path. -- All tests pass. - -Validation: - -```bash -rg -n "data_package\\.json|inspect briefing|briefing\\.Package" internal docs -g '!docs/roadmap/**' -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -This stage is suitable for one implementation prompt. - -## Stage 12: Documentation And Example Alignment - -Goal: align non-roadmap docs with implemented module behavior. - -Files to inspect/update: - -- `README.md`, only if the orientation or quickstart changed; -- `docs/cli.md`; -- `docs/config.md`; -- `docs/operations.md`; -- `docs/troubleshooting.md`; -- `docs/internal/app-orchestration.md`; -- `docs/internal/briefing.md` or replacement module internals doc; -- `docs/internal/changes.md`; -- `docs/internal/forecast-derivation.md`; -- `docs/internal/prompt-input.md`; -- `docs/internal/state.md`; -- `docs/internal/weather-data.md`; -- `docs/integrations/scriptorium.md`; -- `examples/config.yml`. - -Implementation guidance: - -- Document only implemented behavior outside `docs/roadmap/`. -- Add or update an internal module contract document if module behavior is now - implemented. -- Document `inspect modules` and remove `inspect briefing`. -- Document YAML data packages and JSON module snapshots. -- Document report module overrides and typed options only if implemented. -- Keep QPF as future-only unless upstream support was added. -- Keep Scriptorium contract focused on `--input data_package=` and the - actual file format now passed. - -Acceptance criteria: - -- Non-roadmap docs no longer describe old report-shaped briefing packages. -- Config examples load. -- CLI examples match `weatherreporter --help`. -- Docs clearly distinguish module snapshots from prompt data packages. - -Validation: - -```bash -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -rg -n "inspect briefing|data_package\\.json|report-shaped|vars-file|promptvars" README.md docs examples internal -g '!docs/roadmap/**' -``` - -This stage is suitable for one implementation prompt. - -## Stage 13: Final Validation - -Goal: run full validation and catch stale assumptions after the cutover. - -Required commands: - -```bash -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -Required grep checks: - -```bash -rg -n "inspect briefing|data_package\\.json|briefing\\.Package|Daily struct|ThreeDay struct|Weekend struct|Storm struct" internal docs examples -g '!docs/roadmap/**' -rg -n "measurable_qpf_total_in|max_hourly_qpf_in" internal docs examples -g '!docs/roadmap/**' -``` - -Expected grep results: - -- no production-code references to `inspect briefing`; -- no production-code assumption that prompt packages are JSON; -- no production-code dependence on old report-shaped briefing containers; -- QPF references appear only as future-target docs or omitted-field tests until - upstream QPF exists. - -Manual review: - -- Generate command output still writes managed Markdown reports. -- Batch behavior still continues independent reports and returns nonzero on - aggregate failure. -- Distributor notification still uploads the managed Markdown report, not - module snapshots or YAML prompt packages. -- Secrets are not printed or persisted. -- YAML prompt package is readable and contains named stanzas. - -## Deferred Work - -Do not include these in the initial module cutover: - -- plugin architecture; -- dynamic module loading; -- YAML-defined module schemas; -- user-authored module code; -- module-owned Weather API fetching; -- QPF fields before upstream QPF exists; -- SPC modules before upstream SPC data exists; -- radar modules before upstream radar data exists; -- event/storm-review reports unless a separate roadmap implements them. - -## Open Questions - -No blocking open questions remain for this implementation plan. The previously -identified choices are locked above: - -- persist JSON module snapshots and YAML prompt data packages; -- split broad normalized source types into `internal/weatherdata`; -- replace `inspect briefing` with `inspect modules`; -- support typed module options from the first config implementation. diff --git a/docs/roadmap/modules.md b/docs/roadmap/modules.md deleted file mode 100644 index 94c509c..0000000 --- a/docs/roadmap/modules.md +++ /dev/null @@ -1,677 +0,0 @@ -# Modular Data Package Roadmap - -This roadmap describes planned refactoring work that is not implemented. -Current behavior is documented outside `docs/roadmap/`. - -## Purpose - -Move weatherreporter toward deterministic, reusable briefing modules that can -be composed per report type. The goal is to make prompt input easier for the -LLM to understand, easier for operators to inspect, and easier for developers -to change without touching a cross-cutting set of report-builder files. - -The target outcome is a prompt-facing YAML data package with named stanzas. Each -stanza should be built by a self-contained module that derives clear, -deterministic facts from normalized forecast inputs. Reports should choose -modules by ordered module IDs, so experimenting with a report can be as small -as changing one configuration line, plus any matching prompt change outside -weatherreporter. - -The internal target shape is: - -```text -CollectedFacts -> DerivedFacts -> ModuleOutput -``` - -Each arrow should be a stable internal contract. `DerivedFacts` should not need -to know how `CollectedFacts` were collected. Modules should not need to know -the provenance of any collected or derived fact they consume. Report building -should not need to know how a module sourced its underlying facts or calculated -its output. - -## Intent And Context - -The current application already curates source data before passing it to the -LLM. This refactor should strengthen that design. Modules should not expose raw -source complexity merely because it is available. They should compute and -package the facts the LLM should not have to infer from raw hourly periods, -alerts, narrative periods, forecast discussions, or weather stories. - -The desired module behavior is deterministic. A module should answer a narrow -question such as: - -- what are the current conditions; -- what are the key daily forecast facts; -- what are the daypart summaries; -- when is precipitation most likely; -- which alerts overlap the report period; -- what short-term AFD text is relevant; -- what weather story text is relevant. - -The primary maintainability goal is local reasoning. For example, updating the -derived daily summary should mostly involve one module implementation and its -tests. Adding AFD short-term text to a future `next_six_hours` report should be -a report composition change, not a copy/paste change across multiple builders. - -## Target Prompt Shape - -The target prompt-facing data package should be YAML with named stanzas under -`briefing`. Named stanzas are preferred over an array of generic module objects -because they are easier to read, inspect, and reference in prompts. - -Example target shape: - -```yaml -report: - id: daily_today - prompt_id: weather.daily_report - generated_at: 2026-06-09T07:15:00-05:00 - timezone: America/Chicago - current_local_date: 2026-06-09 - valid_period: - start: 2026-06-09T00:00:00-05:00 - end: 2026-06-10T00:00:00-05:00 - -briefing: - metadata: - location: - id: home - name: Brentwood - region: St. Louis Metro - timezone: America/Chicago - - current_conditions: - condition_text: Partly cloudy - temperature_f: 74 - apparent_temperature_f: 76 - dewpoint_f: 66 - relative_humidity_percent: 71 - wind_speed_mph: 8 - wind_direction_degrees: 190 - - derived_daily_summary: - high_temp_f: 86 - low_temp_f: 68 - max_pop_percent: 70 - max_pop_window: "2 PM-6 PM" - measurable_qpf_total_in: 0.35 - max_hourly_qpf_in: 0.12 - first_precip_hour: "1 PM" - last_precip_hour: "8 PM" - thunder_mentioned: true - max_wind_gust_mph: 28 - heat_index_max_f: 91 - - derived_daypart_summaries: - morning: - temp_range_f: "70-78" - max_pop_percent: 20 - dominant_condition: Partly sunny - afternoon: - temp_range_f: "82-86" - max_pop_percent: 70 - dominant_condition: Showers and thunderstorms likely - - alert_digest: - checked: true - active_count: 0 - relevant_count: 0 - - area_forecast_discussion: - key_messages: - - Scattered storms are possible this afternoon. - short_term: Showers and storms increase during the afternoon. - long_term: Periodic rain chances continue into the weekend. - - weather_story: - available: true - title: Several Chances for Rain Through Monday - description: Scattered showers and thunderstorms remain possible. - -recent_changes: - items: [] -``` - -Field names should include units where the unit is not obvious: -`high_temp_f`, `max_pop_percent`, `measurable_qpf_total_in`, -`max_wind_gust_mph`, and similar names are preferred over ambiguous generic -names. Time and range strings should be formatted for prompt readability, while -machine-oriented timestamps should remain available in report metadata. - -The QPF fields in the example are target output fields for a future upstream -source. They should not be treated as immediately implementable from the -current weatherfeeder-backed `CollectedFacts` sources. Until an upstream QPF -source exists, QPF fields should be omitted rather than fabricated from -precipitation probability or narrative text. - -## Architecture Target - -Keep the current package boundaries: - -- `internal/forecast` owns normalized source data, deterministic forecast - derivation, period slicing, daypart grouping, and weather-signal calculations. -- `internal/briefing` owns prompt-facing module builders and module output - schemas. -- `internal/report` owns report identity, prompt ID, valid-period resolution, - output naming, comparison strategy, and default module composition. -- `internal/config` owns optional report module composition overrides. -- `internal/promptinput` owns final data-package assembly, validation, and - prompt-facing serialization. -- `internal/app` remains orchestration: resolve report, fetch sources, build - module context, execute configured modules, persist artifacts, run - Scriptorium, and notify distributor. - -Do not move source fetching, subprocess execution, distributor upload behavior, -or raw external dependency types into module code. - -## Layered Fact Contracts - -Introduce explicit internal contracts for three layers: - -1. `CollectedFacts` -2. `DerivedFacts` -3. `ModuleOutput` - -`CollectedFacts` are normalized upstream inputs collected once per report run. -They should be broad and source-oriented, but not tied to Weather API transport -details. Examples include current conditions, observations, hourly forecast -runs, narrative forecast runs, active alerts, AFD discussion, weather story, -future radar inputs, and future historical observation totals. - -`DerivedFacts` are reusable, report-scoped deterministic products calculated -from `CollectedFacts`. They may slice, combine, group, or summarize collected -facts when the result is broadly useful to more than one module or needed for -consistent behavior across modules. Examples include valid-period hourly -periods, valid-period narrative periods, alert overlaps, daily summaries, -configured daypart summaries, and reusable precipitation timing windows. - -`ModuleOutput` is the prompt-facing output contract produced by one module. A -module may pass through raw-ish facts, such as AFD text, or expose derived -facts, such as daily summary fields. In both cases, the module owns the named -stanza shape and should produce stable, readable, unit-explicit prompt fields. - -Unless implementation discovers a strong reason otherwise, the internal -contract for accessing `CollectedFacts` and `DerivedFacts` should have the same -shape: - -- typed Go structs with named fields; -- nil pointers, empty slices, or zero values to represent absent facts; -- no `map[string]any` or string-keyed fact lookup as the primary API; -- immutable-by-convention values once passed to modules; -- helper methods only for repeated access patterns that would otherwise be - error-prone; -- source provenance and warnings stored separately from the primary fact - values, available to metadata/source-warning modules but not required by - ordinary modules. - -Illustrative shape: - -```go -type CollectedFacts struct { - Current *weatherdata.CurrentConditions - Observations *weatherdata.ObservationRun - Alerts *weatherdata.AlertRun - Hourly *weatherdata.ForecastRun - Narrative *weatherdata.ForecastRun - Discussion *weatherdata.Discussion - WeatherStory *weatherdata.WeatherStory - // Future: radar, historical observations, snow/rain totals, etc. -} - -type DerivedFacts struct { - HourlyPeriods []weatherdata.ForecastPeriod - NarrativePeriods []weatherdata.ForecastPeriod - AlertOverlaps []weatherdata.AlertOverlap - DailySummaries []forecast.DailySummary - DaypartSummaries []forecast.DaypartSummary - PrecipTiming *forecast.PrecipTiming -} - -type ModuleContext struct { - Report report.Resolved - Collected CollectedFacts - Derived DerivedFacts - Units string - Timezone string - Location *LocationContext -} -``` - -The exact package names may differ during implementation. The important -boundary is semantic: `CollectedFacts` represent upstream facts after -normalization; `DerivedFacts` represent reusable report-scoped transformations; -modules represent prompt-facing stanza construction. - -### DerivedFacts Boundary - -Be conservative about what belongs in `DerivedFacts`. Add a value to this layer -only when it is: - -- deterministic; -- report-scoped; -- reusable by multiple modules or needed to keep modules consistent; -- independent of prompt wording and presentation decisions. - -`DerivedFacts` may: - -- slice source periods to the report valid period; -- group hourly data into configured dayparts; -- compute reusable summaries; -- compute alert overlaps; -- normalize repeated time-window selections. - -`DerivedFacts` should not: - -- decide prompt-facing wording; -- decide which facts are important for one module only; -- format prose-like strings for the LLM; -- fetch upstream data; -- write artifacts; -- depend on Scriptorium or distributor. - -Module-specific calculations should remain inside the module when they are -presentation-specific, used by only one module, or likely to change while -tuning prompt behavior. - -## Module Model - -Introduce a typed module model rather than generic maps. A module should have: - -- stable module ID; -- self-contained output struct; -- one focused builder function; -- fixture or unit tests near the module; -- declared input requirements, such as hourly forecast, alerts, discussion, or - weather story; -- deterministic handling for missing optional source data; -- prompt-facing field names that are stable and unit-explicit. - -The implementation may use a simple function registry rather than a broad -interface if that is enough: - -```go -type ModuleID string - -type ModuleBuilder func(ModuleContext) (ModuleOutput, error) -``` - -`ModuleOutput` should include the stable module ID, the YAML stanza name, and a -typed value owned by the module: - -```go -type ModuleOutput struct { - ID ModuleID - StanzaName string - Value any -} -``` - -The module registry should preserve output order from report composition, but -the serialized YAML should use named stanzas for clarity. - -Each module should be able to produce exactly one named stanza. If one source -can usefully feed multiple stanzas, split that into multiple modules rather than -making one module produce unrelated output. - -## Report Composition Target - -Report definitions should declare default ordered module IDs. Configuration may -override the ordered module list for implemented reports. - -Illustrative future config shape: - -```yaml -reports: - next_6_hours: - deterministic_modules: - - hourly_table - - precip_timing - - alert_digest - - afd_short_term_text - - weather_story_text - - spc_products - - daily: - deterministic_modules: - - current_conditions - - derived_daily_summary - - derived_daypart_summaries - - precip_timing - - alert_digest - - forecast_delta - - afd_short_term_text - - weather_story_text - - spc_products -``` - -Configuration should validate unknown module IDs, duplicate module IDs when -duplicates are not meaningful, and modules that are incompatible with the -selected report period. Defaults should remain in Go so the application works -without report composition config. - -## Clean-Break Cutover Policy - -This project is still pre-release. Prefer a direct cutover to the new internal -shape instead of preserving transitional report-shaped briefing structures. - -Implementation should: - -- replace report-shaped briefing containers with module-oriented snapshots; -- replace JSON prompt package output with YAML prompt package output; -- update inspect commands, Recent Changes, tests, and docs in the same cutover; -- remove obsolete `Daily`, `ThreeDay`, `Weekend`, and `Storm` briefing - container shapes when no longer needed; -- avoid compatibility aliases unless they materially reduce implementation - risk inside one stage. - -The public CLI command names, report IDs, prompt IDs, RunID format, managed -Markdown report paths, and distributor upload source should remain stable unless -a separate roadmap explicitly changes them. - -## Artifact And State Target - -The durable artifacts should reflect the new module-oriented model. - -Recommended target: - -- module snapshot artifact: structured JSON for stable inspection, state - lookup, and Recent Changes comparisons; -- prompt data package artifact: YAML with named stanzas, passed to Scriptorium - as `data_package`; -- metadata artifact: JSON linking the module snapshot, YAML data package, - preflight output, rendered report, source warnings, source hashes, and - distributor notification artifact when present. - -The workspace path names should make the artifact type clear. A future -implementation may keep the existing `data-packages/` directory, but file -extensions and metadata fields should reflect the real format, such as: - -```text -workspace/ - snapshots///.modules.json - data-packages///.data_package.yaml -``` - -Replace `inspect briefing` with `inspect modules` during the cutover. -`inspect modules` should return the module snapshot. `inspect data-package` -should return the YAML artifact or a parsed representation of the YAML artifact. -Do not leave inspect commands pointed at obsolete report-shaped data. - -## Module Options And Compatibility - -Each module should have a typed options struct, even when initially empty. -Configuration may decode module options from YAML, but internal module builders -should receive typed options rather than `map[string]any`. - -Illustrative config shape: - -```yaml -reports: - next_6_hours: - deterministic_modules: - - id: hourly_table - options: - range: valid_period - fields: - - time - - temperature_f - - pop_percent - - wind_gust_mph - - id: afd_short_term_text -``` - -Module definitions should declare: - -- module ID; -- stanza name; -- typed options schema; -- supported report IDs or report categories; -- required collected facts; -- required derived facts; -- whether missing optional facts omit the stanza, emit an empty stanza, or - produce a warning; -- whether duplicate use of the module is allowed. - -Configuration validation should reject: - -- unknown report IDs; -- unknown module IDs; -- duplicate module IDs unless explicitly allowed; -- two modules that render the same stanza name; -- module options that do not match the module's typed option schema; -- modules that are incompatible with the report's valid-period strategy or - available facts. - -## Recent Changes Target - -Recent Changes must remain structured and deterministic. During the clean-break -cutover, move comparison inputs away from report-shaped `briefing.Package` -values and toward module-oriented snapshots. - -Recommended target: - -- compare `ModuleOutput` values or typed module snapshot stanzas, not rendered - YAML and not rendered Markdown; -- keep report-compatible matching policy in `internal/report`; -- keep threshold configuration in `internal/config`; -- keep comparison algorithms in `internal/changes`; -- make each comparison explicit about which module stanzas it needs. - -For example, Daily comparison should primarily consume -`derived_daily_summary`, `derived_daypart_summaries`, `alert_digest`, and -`precip_timing` if present. If a required stanza is missing, the comparison -should return no change with an inspectable warning or an actionable error, -depending on the report's configured missing-data policy. - -## Package Naming Target - -The current `internal/forecast` package owns both forecast-specific derivation -and broader normalized weather data. Because planned sources include current -observations, radar, and historical review inputs, implementation should -consider splitting names during the clean-break refactor: - -- `internal/weatherdata`: normalized collected source facts, source metadata, - warnings, and broad weather data types; -- `internal/forecast`: forecast-specific algorithms such as period slicing, - daily summaries, daypart grouping, and precipitation timing. - -If this split is too large for the first cutover, introduce `CollectedFacts` in -the package that minimizes churn, but avoid expanding the meaning of -`internal/forecast` further in new module contracts. - -## Initial Module Candidates - -The first module catalog should start with the modules needed to replace current -report-shaped briefing output and should clearly distinguish implemented -modules from future-only candidates. - -Initial candidates: - -- `metadata` -- `current_conditions` -- `derived_daily_summary` -- `derived_daypart_summaries` -- `hourly_table` -- `precip_timing` -- `alert_digest` -- `area_forecast_discussion` -- `afd_key_messages` -- `afd_short_term_text` -- `afd_long_term_text` -- `weather_story` -- `forecast_delta` -- `outdoor_windows` -- `weekend_planning` -- `storm_window_summary` - -Each module should declare inputs, outputs, report applicability, -missing-data behavior, compatibility behavior, and options. - -## Target Derived Daily Summary - -The intended `derived_daily_summary` shape is: - -```yaml -derived_daily_summary: - high_temp_f: 86 - low_temp_f: 68 - max_pop_percent: 70 - max_pop_window: "2 PM-6 PM" - measurable_qpf_total_in: 0.35 - max_hourly_qpf_in: 0.12 - first_precip_hour: "1 PM" - last_precip_hour: "8 PM" - thunder_mentioned: true - max_wind_gust_mph: 28 - heat_index_max_f: 91 -``` - -`measurable_qpf_total_in` and `max_hourly_qpf_in` are future target fields. -They require a real upstream quantitative precipitation source and should be -omitted until such a source is represented in `CollectedFacts`. - -## Target Derived Daypart Summaries - -The intended `derived_daypart_summaries` shape is: - -```yaml -derived_daypart_summaries: - morning: - temp_range_f: "70-78" - max_pop_percent: 20 - dominant_condition: Partly sunny - afternoon: - temp_range_f: "82-86" - max_pop_percent: 70 - dominant_condition: Showers and thunderstorms likely -``` - -## Configurable Composition Target - -The target configuration model should allow report module composition to be -changed without editing cross-cutting report-builder code. Built-in defaults -should remain in Go so the application works with no module override config. - -Illustrative config: - -```yaml -reports: - daily: - deterministic_modules: - - current_conditions - - derived_daily_summary - - derived_daypart_summaries - - precip_timing - - alert_digest - - afd_short_term_text - - weather_story_text -``` - -Adding or removing an implemented module from an implemented report should be a -single config edit. Unknown modules, invalid options, duplicate stanzas, and -incompatible report/module combinations should fail with actionable errors. - -## Acceptance Criteria - -The refactor is complete when: - -- current implemented reports generate successfully from named-stanza YAML - prompt packages; -- module snapshots are persisted as structured JSON and linked from metadata; -- `inspect modules` returns module snapshots; -- `inspect briefing` is removed from CLI help, parser support, and - non-roadmap docs; -- Recent Changes compares structured module snapshots, not rendered Markdown or - YAML text; -- report definitions declare default module order in one place; -- implemented report module composition can be overridden by config; -- implemented modules have typed options and compatibility contracts; -- `CollectedFacts` are built once per report run and reused by all modules; -- `DerivedFacts` are built from `CollectedFacts` and do not depend on adapter - transport details; -- module builders do not call Weather API, Scriptorium, distributor, or - filesystem state directly; -- stale report-shaped briefing containers are removed from the generation path; -- QPF output fields remain omitted until upstream QPF exists. - -## Design Rules - -- Keep modules deterministic. -- Keep modules self-contained where practical. -- Preserve the `CollectedFacts -> DerivedFacts -> ModuleOutput` boundary. -- Build `CollectedFacts` once per report run. -- Build `DerivedFacts` from `CollectedFacts`, not from adapter-specific - transport details. -- Keep `DerivedFacts` conservative and reusable. -- Keep raw external source details behind adapters and forecast normalization. -- Keep report composition centralized and ordered. -- Prefer typed outputs over generic maps. -- Prefer typed fact contracts over string-keyed fact registries. -- Prefer named YAML stanzas over generic module arrays. -- Use unit-explicit field names. -- Do not require the LLM to calculate obvious derived facts. -- Do not let module builders call external services or write durable state. -- Do not introduce plugins, dynamic loading, or a generic workflow engine. - -## Risks And Mitigations - -- Prompt contract churn: stage YAML introduction after module outputs are - tested and inspectable. -- Recent Changes drift: compare stable module outputs and keep snapshot tests. -- Over-abstraction: start with simple builders and a registry, not a framework. -- Config complexity: expose ordered module selection and typed options only for - implemented modules; defer broad parameterization. -- Loss of useful context: preserve focused source excerpts and source warnings, - but avoid reintroducing raw unbounded payloads. - -## Deferred Work - -These are out of scope for the initial module refactor: - -- dynamic plugin loading; -- user-defined module code; -- YAML-defined module schemas; -- module parameterization beyond implemented typed options; -- replacing Weather API source fetching with module-owned fetches; -- moving prompt authoring or Scriptorium prompt changes into weatherreporter; -- adding future source modules, such as SPC products, before upstream data and - report requirements exist. - -## Open Questions - -### Should module snapshots and prompt packages both be persisted? - -Recommended approach: persist module snapshots as JSON and prompt packages as -YAML. JSON module snapshots are better for structured Recent Changes, state -lookup, and tests. YAML prompt packages are better for prompt readability and -LLM consumption. Keeping both artifacts gives each use case the right format -without asking comparison code to parse prompt-oriented YAML. - -Viable alternative: persist only the YAML prompt package and parse it for -inspection and Recent Changes. This reduces artifact count, but it couples -machine comparison to prompt formatting and makes future prompt-oriented -formatting changes riskier. - -### Should `internal/forecast` be split during the first cutover? - -Recommended approach: split broad normalized source types into -`internal/weatherdata` during the clean-break cutover if the implementation -scope remains manageable. This name fits current conditions, alerts, -discussion, weather story, future radar, and future historical data better than -`forecast`. - -Viable alternative: keep existing `internal/forecast` types for the first -module implementation and introduce `CollectedFacts` as a wrapper. This reduces -short-term churn, but it leaves a package name that will become increasingly -misleading as non-forecast sources grow. - -### Should module config support typed options immediately? - -Recommended approach: support typed options for implemented modules from the -start, even if most modules use empty options. This establishes the extension -point needed for hourly ranges, field selection, and AFD section choices -without adding dynamic maps to module builders. - -Viable alternative: initially support only ordered module IDs and add options -later. This is simpler, but it may force another config shape change as soon as -hourly range or section-selection experiments begin. diff --git a/examples/config.yml b/examples/config.yml index 50374c7..260dcc0 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -76,7 +76,6 @@ reports: - derived_daypart_summaries - precip_timing - alert_digest - - forecast_delta - id: area_forecast_discussion options: sections: diff --git a/internal/app/app.go b/internal/app/app.go index 3c1d905..8f4a921 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -827,13 +827,6 @@ func BuildModuleSnapshotFromFacts(req ModuleSnapshotRequest, reportFacts ReportF } var outputs []module.Output for _, item := range req.Resolved.Definition.Modules { - definition, err := registry.Lookup(item.ID) - if err != nil { - return module.Snapshot{}, err - } - if definition.Builder == nil { - continue - } output, err := registry.BuildModule(moduleContext, item) if err != nil { return module.Snapshot{}, err diff --git a/internal/briefing/modules.go b/internal/briefing/modules.go index 081c1d0..9edbeae 100644 --- a/internal/briefing/modules.go +++ b/internal/briefing/modules.go @@ -3,6 +3,7 @@ package briefing import ( "fmt" "reflect" + "strings" "gitea.maximumdirect.net/eric/weatherreporter/internal/facts" "gitea.maximumdirect.net/eric/weatherreporter/internal/module" @@ -61,6 +62,12 @@ func NewModuleRegistry(definitions []ModuleDefinition) (ModuleRegistry, error) { if _, ok := registry.definitions[definition.ID]; ok { return ModuleRegistry{}, fmt.Errorf("duplicate module definition %q", definition.ID) } + if definition.Builder == nil { + return ModuleRegistry{}, fmt.Errorf("module %q has no builder", definition.ID) + } + if definition.MissingData == module.MissingDataWarn { + return ModuleRegistry{}, fmt.Errorf("module %q uses unsupported missing data behavior %q", definition.ID, definition.MissingData) + } if existingID, ok := seenStanzas[definition.StanzaName]; ok { return ModuleRegistry{}, fmt.Errorf("duplicate stanza name %q for modules %q and %q", definition.StanzaName, existingID, definition.ID) } @@ -92,6 +99,20 @@ func (r ModuleRegistry) BuildModule(ctx ModuleContext, item module.ConfigItem) ( if definition.Builder == nil { return nil, fmt.Errorf("module %q has no builder", item.ID) } + missing := missingRequirements(definition, ctx) + if len(missing) > 0 { + switch definition.MissingData { + case module.MissingDataOmit: + return nil, nil + case module.MissingDataError: + return nil, fmt.Errorf("module %q missing required facts: %s", item.ID, strings.Join(missing, ", ")) + case module.MissingDataEmpty: + case module.MissingDataWarn: + return nil, fmt.Errorf("module %q uses unsupported missing data behavior %q", item.ID, definition.MissingData) + default: + return nil, fmt.Errorf("module %q has unknown missing data behavior %q", item.ID, definition.MissingData) + } + } options := item.Options if options == nil { options = definition.DefaultOptions @@ -112,6 +133,57 @@ func (r ModuleRegistry) BuildModule(ctx ModuleContext, item module.ConfigItem) ( return output, nil } +func missingRequirements(definition ModuleDefinition, ctx ModuleContext) []string { + var missing []string + for _, requirement := range definition.RequiredCollected { + if !collectedFactAvailable(requirement, ctx) { + missing = append(missing, string(requirement)) + } + } + for _, requirement := range definition.RequiredDerived { + if !derivedFactAvailable(requirement, ctx) { + missing = append(missing, string(requirement)) + } + } + return missing +} + +func collectedFactAvailable(requirement module.FactRequirement, ctx ModuleContext) bool { + switch requirement { + case module.CollectedCurrentConditions: + return ctx.Collected.Current != nil + case module.CollectedAlerts: + return ctx.Collected.Alerts != nil + case module.CollectedDiscussion: + return ctx.Collected.Discussion != nil + case module.CollectedWeatherStory: + return ctx.Collected.WeatherStory != nil + case module.CollectedSourceMetadata: + return len(ctx.Collected.SourceProvenance) > 0 || len(ctx.Collected.SourceWarnings) > 0 + default: + return false + } +} + +func derivedFactAvailable(requirement module.FactRequirement, ctx ModuleContext) bool { + switch requirement { + case module.RequiresDerivedHourlyPeriods: + return len(ctx.Derived.ValidPeriodHourlyPeriods) > 0 + case module.RequiresDerivedNarrativePeriods: + return len(ctx.Derived.ValidPeriodNarrativePeriods) > 0 + case module.RequiresDerivedAlertOverlaps: + return true + case module.RequiresDerivedDailySummaries: + return len(ctx.Derived.DailySummaries) > 0 + case module.RequiresDerivedDaypartSummaries: + return len(ctx.Derived.DaypartSummaries) > 0 + case module.RequiresDerivedPrecipTiming: + return true + default: + return false + } +} + func (r ModuleRegistry) ValidateComposition(reportID report.ID, items []module.ConfigItem) error { seenModules := map[module.ID]struct{}{} seenStanzas := map[string]module.ID{} @@ -208,14 +280,6 @@ func defaultModuleDefinitions() []ModuleDefinition { MissingData: module.MissingDataError, Builder: buildDerivedDaypartSummariesModule, }, - { - ID: module.HourlyTable, - StanzaName: "hourly_table", - DefaultOptions: module.HourlyTableOptions{}, - RequiredDerived: []module.FactRequirement{module.RequiresDerivedHourlyPeriods}, - SupportedReports: []report.ID{report.Storm}, - MissingData: module.MissingDataError, - }, { ID: module.PrecipTiming, StanzaName: "precip_timing", @@ -253,13 +317,6 @@ func defaultModuleDefinitions() []ModuleDefinition { MissingData: module.MissingDataOmit, Builder: buildWeatherStoryModule, }, - { - ID: module.ForecastDelta, - StanzaName: "forecast_delta", - DefaultOptions: module.ForecastDeltaOptions{}, - SupportedReports: []report.ID{report.DailyToday, report.DailyTomorrow, report.ThreeDay}, - MissingData: module.MissingDataEmpty, - }, { ID: module.OutdoorWindows, StanzaName: "outdoor_windows", @@ -278,21 +335,5 @@ func defaultModuleDefinitions() []ModuleDefinition { MissingData: module.MissingDataEmpty, Builder: buildTomorrowPlanningModule, }, - { - ID: module.WeekendPlanning, - StanzaName: "weekend_planning", - DefaultOptions: module.WeekendPlanningOptions{}, - RequiredDerived: []module.FactRequirement{module.RequiresDerivedDailySummaries}, - SupportedReports: []report.ID{report.Weekend}, - MissingData: module.MissingDataEmpty, - }, - { - ID: module.StormWindowSummary, - StanzaName: "storm_window_summary", - DefaultOptions: module.StormWindowSummaryOptions{}, - RequiredDerived: []module.FactRequirement{module.RequiresDerivedStormWindowSummary}, - SupportedReports: []report.ID{report.Storm}, - MissingData: module.MissingDataError, - }, } } diff --git a/internal/briefing/modules_test.go b/internal/briefing/modules_test.go index 282d54d..9717895 100644 --- a/internal/briefing/modules_test.go +++ b/internal/briefing/modules_test.go @@ -14,6 +14,41 @@ func TestDefaultModuleRegistryValidatesReportDefaults(t *testing.T) { if err := registry.ValidateComposition(definition.ID, definition.Modules); err != nil { t.Fatalf("ValidateComposition(%s) error = %v", definition.ID, err) } + for _, item := range definition.Modules { + moduleDefinition, err := registry.Lookup(item.ID) + if err != nil { + t.Fatalf("Lookup(%s) error = %v", item.ID, err) + } + if moduleDefinition.Builder == nil { + t.Fatalf("report %s module %s has no builder", definition.ID, item.ID) + } + } + } +} + +func TestDefaultReportModulesBuildSnapshots(t *testing.T) { + registry := MustDefaultModuleRegistry() + for _, definition := range report.DefaultRegistry().All() { + t.Run(string(definition.ID), func(t *testing.T) { + ctx := derivedModuleContext(definition.ID) + var outputs []module.Output + for _, item := range definition.Modules { + output, err := registry.BuildModule(ctx, item) + if err != nil { + t.Fatalf("BuildModule(%s) error = %v", item.ID, err) + } + if output != nil { + outputs = append(outputs, *output) + } + } + snapshot, err := module.NewSnapshot(outputs) + if err != nil { + t.Fatalf("NewSnapshot() error = %v", err) + } + if len(snapshot.Outputs) == 0 { + t.Fatal("snapshot outputs = 0, want default report modules") + } + }) } } @@ -38,8 +73,8 @@ func TestModuleRegistryRejectsDuplicateModuleIDs(t *testing.T) { func TestModuleRegistryRejectsDuplicateStanzaNames(t *testing.T) { _, err := NewModuleRegistry([]ModuleDefinition{ - {ID: module.Metadata, StanzaName: "metadata", DefaultOptions: module.MetadataOptions{}}, - {ID: module.CurrentConditions, StanzaName: "metadata", DefaultOptions: module.CurrentConditionsOptions{}}, + {ID: module.Metadata, StanzaName: "metadata", DefaultOptions: module.MetadataOptions{}, Builder: noopModuleBuilder}, + {ID: module.CurrentConditions, StanzaName: "metadata", DefaultOptions: module.CurrentConditionsOptions{}, Builder: noopModuleBuilder}, }) if err == nil || !strings.Contains(err.Error(), `duplicate stanza name "metadata"`) { t.Fatalf("error = %v, want duplicate stanza name", err) @@ -48,12 +83,30 @@ func TestModuleRegistryRejectsDuplicateStanzaNames(t *testing.T) { func TestModuleRegistryRejectsIncompatibleReports(t *testing.T) { registry := MustDefaultModuleRegistry() - err := registry.ValidateComposition(report.DailyToday, []module.ConfigItem{{ID: module.StormWindowSummary}}) - if err == nil || !strings.Contains(err.Error(), `module "storm_window_summary" is not compatible with report "daily_today"`) { + err := registry.ValidateComposition(report.DailyToday, []module.ConfigItem{{ID: module.TomorrowPlanning}}) + if err == nil || !strings.Contains(err.Error(), `module "tomorrow_planning" is not compatible with report "daily_today"`) { t.Fatalf("error = %v, want incompatible report", err) } } +func TestModuleRegistryRejectsDefinitionsWithoutBuilders(t *testing.T) { + _, err := NewModuleRegistry([]ModuleDefinition{ + {ID: module.Metadata, StanzaName: "metadata", DefaultOptions: module.MetadataOptions{}}, + }) + if err == nil || !strings.Contains(err.Error(), `module "metadata" has no builder`) { + t.Fatalf("error = %v, want missing builder", err) + } +} + +func TestModuleRegistryRejectsUnsupportedMissingDataWarn(t *testing.T) { + _, err := NewModuleRegistry([]ModuleDefinition{ + {ID: module.Metadata, StanzaName: "metadata", DefaultOptions: module.MetadataOptions{}, MissingData: module.MissingDataWarn, Builder: noopModuleBuilder}, + }) + if err == nil || !strings.Contains(err.Error(), `unsupported missing data behavior`) { + t.Fatalf("error = %v, want unsupported missing-data behavior", err) + } +} + func TestModuleRegistryRejectsInvalidOptionShapes(t *testing.T) { registry := MustDefaultModuleRegistry() err := registry.ValidateComposition(report.DailyToday, []module.ConfigItem{ @@ -74,3 +127,7 @@ func TestModuleRegistryAcceptsTypedOptions(t *testing.T) { t.Fatalf("ValidateComposition() error = %v", err) } } + +func noopModuleBuilder(ModuleContext, any) (*module.Output, error) { + return &module.Output{ID: module.Metadata, StanzaName: "metadata", Value: struct{}{}}, nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5cc5fd4..f616552 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -196,10 +196,20 @@ reports: reports: daily: deterministic_modules: - - storm_window_summary + - tomorrow_planning `, wantErr: `not compatible with report "daily_today"`, }, + { + name: "RemovedPlaceholderModule", + yaml: ` +reports: + daily: + deterministic_modules: + - forecast_delta +`, + wantErr: `unknown module "forecast_delta"`, + }, { name: "InvalidOptions", yaml: ` diff --git a/internal/module/module.go b/internal/module/module.go index 91abb61..c6792c0 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -15,16 +15,12 @@ const ( CurrentConditions ID = "current_conditions" DerivedDailySummary ID = "derived_daily_summary" DerivedDaypartSummaries ID = "derived_daypart_summaries" - HourlyTable ID = "hourly_table" PrecipTiming ID = "precip_timing" AlertDigest ID = "alert_digest" AreaForecastDiscussion ID = "area_forecast_discussion" WeatherStory ID = "weather_story" - ForecastDelta ID = "forecast_delta" OutdoorWindows ID = "outdoor_windows" TomorrowPlanning ID = "tomorrow_planning" - WeekendPlanning ID = "weekend_planning" - StormWindowSummary ID = "storm_window_summary" ) type ConfigItem struct { @@ -107,18 +103,17 @@ func StanzaValue[T any](s Snapshot, name string) (T, bool, error) { type FactRequirement string const ( - CollectedCurrentConditions FactRequirement = "collected.current_conditions" - CollectedAlerts FactRequirement = "collected.alerts" - CollectedDiscussion FactRequirement = "collected.discussion" - CollectedWeatherStory FactRequirement = "collected.weather_story" - CollectedSourceMetadata FactRequirement = "collected.source_metadata" - RequiresDerivedHourlyPeriods FactRequirement = "derived.hourly_periods" - RequiresDerivedNarrativePeriods FactRequirement = "derived.narrative_periods" - RequiresDerivedAlertOverlaps FactRequirement = "derived.alert_overlaps" - RequiresDerivedDailySummaries FactRequirement = "derived.daily_summaries" - RequiresDerivedDaypartSummaries FactRequirement = "derived.daypart_summaries" - RequiresDerivedPrecipTiming FactRequirement = "derived.precip_timing" - RequiresDerivedStormWindowSummary FactRequirement = "derived.storm_window_summary" + CollectedCurrentConditions FactRequirement = "collected.current_conditions" + CollectedAlerts FactRequirement = "collected.alerts" + CollectedDiscussion FactRequirement = "collected.discussion" + CollectedWeatherStory FactRequirement = "collected.weather_story" + CollectedSourceMetadata FactRequirement = "collected.source_metadata" + RequiresDerivedHourlyPeriods FactRequirement = "derived.hourly_periods" + RequiresDerivedNarrativePeriods FactRequirement = "derived.narrative_periods" + RequiresDerivedAlertOverlaps FactRequirement = "derived.alert_overlaps" + RequiresDerivedDailySummaries FactRequirement = "derived.daily_summaries" + RequiresDerivedDaypartSummaries FactRequirement = "derived.daypart_summaries" + RequiresDerivedPrecipTiming FactRequirement = "derived.precip_timing" ) type MissingDataBehavior string @@ -134,15 +129,11 @@ type MetadataOptions struct{} type CurrentConditionsOptions struct{} type DerivedDailySummaryOptions struct{} type DerivedDaypartSummariesOptions struct{} -type HourlyTableOptions struct{} type PrecipTimingOptions struct{} type AlertDigestOptions struct{} type AreaForecastDiscussionOptions struct { Sections []string `json:"sections,omitempty" yaml:"sections,omitempty"` } type WeatherStoryOptions struct{} -type ForecastDeltaOptions struct{} type OutdoorWindowsOptions struct{} type TomorrowPlanningOptions struct{} -type WeekendPlanningOptions struct{} -type StormWindowSummaryOptions struct{} diff --git a/internal/promptinput/package_test.go b/internal/promptinput/package_test.go index 79e0ef1..9896ec4 100644 --- a/internal/promptinput/package_test.go +++ b/internal/promptinput/package_test.go @@ -106,7 +106,7 @@ func TestBuildUsesNamedSnapshotStanzas(t *testing.T) { req.Metadata.PromptID = "weather.three_day_outlook" req.Modules = snapshotWithOutputs(t, module.Output{ID: module.Metadata, StanzaName: "metadata", Value: map[string]string{"run_id": req.Metadata.RunID}}, - module.Output{ID: module.ForecastDelta, StanzaName: "three_day", Value: map[string]any{"days": []string{"2026-05-29"}}}, + module.Output{ID: module.DerivedDaypartSummaries, StanzaName: "three_day", Value: map[string]any{"days": []string{"2026-05-29"}}}, ) pkg, err := Build(req) diff --git a/internal/report/period_test.go b/internal/report/period_test.go index d2a988a..3cc249a 100644 --- a/internal/report/period_test.go +++ b/internal/report/period_test.go @@ -266,7 +266,6 @@ func TestRegistryDefinitionsDeclareDefaultModules(t *testing.T) { module.DerivedDaypartSummaries, module.PrecipTiming, module.AlertDigest, - module.ForecastDelta, module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, @@ -281,7 +280,6 @@ func TestRegistryDefinitionsDeclareDefaultModules(t *testing.T) { module.DerivedDaypartSummaries, module.PrecipTiming, module.AlertDigest, - module.ForecastDelta, module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, @@ -296,7 +294,6 @@ func TestRegistryDefinitionsDeclareDefaultModules(t *testing.T) { module.DerivedDaypartSummaries, module.PrecipTiming, module.AlertDigest, - module.ForecastDelta, module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, @@ -313,7 +310,6 @@ func TestRegistryDefinitionsDeclareDefaultModules(t *testing.T) { module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, - module.WeekendPlanning, }, }, { @@ -321,12 +317,10 @@ func TestRegistryDefinitionsDeclareDefaultModules(t *testing.T) { want: []module.ID{ module.Metadata, module.CurrentConditions, - module.HourlyTable, module.PrecipTiming, module.AlertDigest, module.AreaForecastDiscussion, module.WeatherStory, - module.StormWindowSummary, }, }, } diff --git a/internal/report/registry.go b/internal/report/registry.go index d6ee443..967e928 100644 --- a/internal/report/registry.go +++ b/internal/report/registry.go @@ -109,7 +109,6 @@ func dailyTodayModules() []module.ConfigItem { module.DerivedDaypartSummaries, module.PrecipTiming, module.AlertDigest, - module.ForecastDelta, module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, @@ -129,7 +128,6 @@ func threeDayModules() []module.ConfigItem { module.DerivedDaypartSummaries, module.PrecipTiming, module.AlertDigest, - module.ForecastDelta, module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, @@ -146,7 +144,6 @@ func weekendModules() []module.ConfigItem { module.AreaForecastDiscussion, module.WeatherStory, module.OutdoorWindows, - module.WeekendPlanning, ) } @@ -154,12 +151,10 @@ func stormModules() []module.ConfigItem { return moduleItems( module.Metadata, module.CurrentConditions, - module.HourlyTable, module.PrecipTiming, module.AlertDigest, module.AreaForecastDiscussion, module.WeatherStory, - module.StormWindowSummary, ) }