Compare commits
2 Commits
d90801cff5
...
0884eb0ce5
| Author | SHA1 | Date | |
|---|---|---|---|
| 0884eb0ce5 | |||
| a27e870522 |
659
docs/roadmap/audit.md
Normal file
659
docs/roadmap/audit.md
Normal file
@@ -0,0 +1,659 @@
|
||||
# Code Quality And Deduplication Audit
|
||||
|
||||
## Executive summary
|
||||
|
||||
Overall code quality is strong. The repository is modular, the important
|
||||
external boundaries are mostly contained behind adapters, state paths are
|
||||
centralized, durable writes use shared file helpers, and report/module identity
|
||||
policy is mostly registry-driven.
|
||||
|
||||
The top three cleanup targets before the next major release are:
|
||||
|
||||
1. Day-style generated-text report duplication across Daily, Today, and
|
||||
Tomorrow.
|
||||
2. Duplicated report-module config normalization and validation logic.
|
||||
3. Repeated Scriptorium run and structured-run result plumbing.
|
||||
|
||||
The codebase appears ready for a limited cleanup pass. I do not see a major
|
||||
architectural risk that would require a broad rewrite. The most useful cleanup
|
||||
work is narrow, behavior-preserving, and should keep public CLI syntax and
|
||||
managed artifact paths stable.
|
||||
|
||||
## Repository map reviewed
|
||||
|
||||
Reviewed documentation and examples:
|
||||
|
||||
- `README.md`
|
||||
- `docs/policy/architecture.md`
|
||||
- `docs/policy/development.md`
|
||||
- `docs/policy/documentation.md`
|
||||
- `docs/config.md`
|
||||
- `docs/cli.md`
|
||||
- `docs/operations.md`
|
||||
- `docs/troubleshooting.md`
|
||||
- `docs/internal/*.md`
|
||||
- `docs/integrations/{weatherapi,scriptorium,distributor}`
|
||||
- `docs/roadmap/{daily,data-package-exports,future,implementation}.md`
|
||||
- `examples/config.yml`
|
||||
- `examples/minimal-config.yml`
|
||||
|
||||
Reviewed code areas:
|
||||
|
||||
- `cmd/weatherreporter`
|
||||
- `internal/cli`
|
||||
- `internal/config`
|
||||
- `internal/app`
|
||||
- `internal/report`
|
||||
- `internal/module`
|
||||
- `internal/briefing`
|
||||
- `internal/facts`
|
||||
- `internal/forecast`
|
||||
- `internal/generatedtext`
|
||||
- `internal/reporttemplate`
|
||||
- `internal/promptinput`
|
||||
- `internal/changes`
|
||||
- `internal/state`
|
||||
- `internal/fileutil`
|
||||
- `internal/adapters/{weatherapi,scriptorium,distributor}`
|
||||
- `internal/weatherdata`
|
||||
- package-level tests throughout `internal`
|
||||
|
||||
Major execution paths reviewed:
|
||||
|
||||
- `weatherreporter generate ...` parsing and orchestration.
|
||||
- `weatherreporter run morning/evening` batch orchestration and output.
|
||||
- `weatherreporter inspect ...` artifact inspection.
|
||||
- Weather API bundle fan-out and missing-source handling.
|
||||
- module snapshot and prompt-input construction.
|
||||
- generated-text-template workflow for `daily`, `today`, `tomorrow`, and
|
||||
`hourly`.
|
||||
- Scriptorium render/run/structured-run calls.
|
||||
- distributor notification request construction, upload, status, and debug
|
||||
artifact persistence.
|
||||
|
||||
Areas not deeply inspected:
|
||||
|
||||
- Historical files under `workspace/`; these are generated artifacts rather
|
||||
than source of truth.
|
||||
- Full live integration behavior against Weather API, Scriptorium, or
|
||||
distributor; tests and adapter code were inspected instead.
|
||||
|
||||
## High-confidence deduplication opportunities
|
||||
|
||||
### 1. Consolidate common Daily/Today/Tomorrow generated-text report plumbing
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/generatedtext/daily.go`
|
||||
- `internal/generatedtext/today.go`
|
||||
- `internal/generatedtext/tomorrow.go`
|
||||
- `internal/generatedtext/render_context.go`
|
||||
- `internal/reporttemplate/templates/daily.md.tmpl`
|
||||
- `internal/reporttemplate/templates/today.md.tmpl`
|
||||
- `internal/reporttemplate/templates/tomorrow.md.tmpl`
|
||||
- `internal/reporttemplate/schemas/*.generated_text.schema.json`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- `Daily`, `Today`, and `Tomorrow` have the same GeneratedText JSON shape:
|
||||
`summary`, array-valued `forecast_discussion`, optional
|
||||
`precipitation_timing`, and optional `confidence`.
|
||||
- `ValidateDaily`, `ValidateToday`, and `ValidateTomorrow` perform the same
|
||||
trim, required-field, normalize, and error-shaping steps with only the report
|
||||
name changing.
|
||||
- `DailyReportContext`, `TodayReportContext`, and `TomorrowReportContext` have
|
||||
the same fields.
|
||||
- `dailyTemplateModules`, `todayTemplateModules`, and
|
||||
`tomorrowTemplateModules` repeat the same module-snapshot lookups for common
|
||||
stanzas, then differ only in ordered daypart wrapper and planning module.
|
||||
- The Daily, Today, and Tomorrow Markdown templates repeat large daypart and
|
||||
precipitation-timing blocks. For example, the Daypart Forecast logic appears
|
||||
in all three templates, with small policy differences such as Today omitting
|
||||
missing daypart lines while Daily/Tomorrow render `Forecast details are
|
||||
limited`.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- These reports are intentionally independent at the public/report-definition
|
||||
layer, but their current internal GeneratedText and daypart template surfaces
|
||||
are close enough that wording, validation, and render-context changes are
|
||||
likely to require edits in multiple places.
|
||||
- Small drift in deterministic report wording is easy to introduce accidentally
|
||||
because the duplicated template logic is dense and hard to compare.
|
||||
- New day-style reports would likely copy this pattern again.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Keep separate exported report-specific types and template files, because the
|
||||
reports are intended to diverge.
|
||||
- Introduce a small shared day-style GeneratedText validation helper that takes
|
||||
the report name and typed value, or introduce an unexported common
|
||||
`dayGeneratedText` shape embedded by report-specific types if that keeps
|
||||
JSON and schema behavior clear.
|
||||
- Add a shared `dayReportCommonModules` or equivalent helper that loads common
|
||||
module stanzas once and lets Daily/Today/Tomorrow add only report-specific
|
||||
planning fields and daypart wrapper types.
|
||||
- Consider named template partials or a small deterministic presentation struct
|
||||
for shared daypart/precipitation line rendering. Do not collapse the separate
|
||||
report template files; make the shared pieces explicit and opt-in.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Keep existing `internal/generatedtext` validation tests for each public type.
|
||||
- Add a table test that Daily, Today, and Tomorrow validators share the same
|
||||
required-field behavior and unknown-field rejection.
|
||||
- Add render-context tests proving common module fields are populated for all
|
||||
three day-style reports and each report still exposes its own planning module.
|
||||
- Add template tests that pin the intended daypart behavior for all three
|
||||
reports after shared rendering cleanup.
|
||||
|
||||
Risk level: medium. The behavior is user-facing Markdown and prompt-schema
|
||||
related, but the cleanup can be done in small steps with existing tests.
|
||||
|
||||
### 2. Centralize report-module config normalization and validation
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/config/reports.go`
|
||||
- `internal/config/config_test.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- `normalizeReportModules` and `validateReportModules` both traverse
|
||||
`cfg.Reports`, resolve config keys with `report.IDForConfigKey`, track
|
||||
duplicate report aliases, initialize the default report and module
|
||||
registries, normalize module options, and call
|
||||
`ModuleRegistry.ValidateComposition`.
|
||||
- `Config.ReportModuleOverrides` performs another report-key and duplicate
|
||||
alias traversal before returning normalized module items.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- Report-module configuration is user-visible policy. Drift between load-time
|
||||
normalization, validation, and app-time override extraction could cause a
|
||||
config to pass one path and fail another, or produce different error wording.
|
||||
- Adding a new report key, module option type, or validation rule currently
|
||||
requires touching and reasoning about multiple similar loops.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Introduce one unexported helper that resolves report config entries into a
|
||||
canonical structure, for example `normalizedReportOverrides(cfg,
|
||||
normalizeOptions bool)`.
|
||||
- Have `Load` call the helper in mutating mode and have `Validate` or
|
||||
`ReportModuleOverrides` reuse the same canonical traversal. Preserve current
|
||||
error prefixes such as `reports.<key>.deterministic_modules[...]`.
|
||||
- Keep config loading and validation in `internal/config`; do not move user
|
||||
config parsing into `internal/app`.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Existing config tests for unknown report keys, duplicate report aliases,
|
||||
unknown modules, incompatible modules, duplicate modules, and invalid module
|
||||
options should continue passing.
|
||||
- Add one regression test that a manually constructed `config.Config` with raw
|
||||
module options fails or normalizes consistently through the same helper path.
|
||||
- Add table coverage that duplicate aliases produce identical error context from
|
||||
`Load` and `ReportModuleOverrides`.
|
||||
|
||||
Risk level: low to medium. This is internal config plumbing, but it protects a
|
||||
public configuration surface.
|
||||
|
||||
### 3. Deduplicate Scriptorium run and structured-run execution plumbing
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/adapters/scriptorium/runner.go`
|
||||
- `internal/adapters/scriptorium/runner_test.go`
|
||||
- `internal/app/app.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- `RunResult` and `StructuredRunResult` have the same fields.
|
||||
- `Runner.Run` and `Runner.StructuredRun` validate the same request fields,
|
||||
call `execute`, build the same result shape, attach `OutputPath`, and check a
|
||||
nonzero exit code. The only meaningful differences are request/result types
|
||||
and error wording.
|
||||
- `structuredRunArgs` delegates to `runArgs`, so the argv behavior is already
|
||||
shared.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- `scriptorium run` is a critical external integration. Any future change to
|
||||
stdout/stderr truncation, exit-code handling, or output-path recording should
|
||||
not require repeated edits.
|
||||
- The duplication is small now, but it is exactly the kind of adapter behavior
|
||||
that can drift invisibly.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Keep the public `RunResult` and `StructuredRunResult` types if callers benefit
|
||||
from distinct names.
|
||||
- Add an unexported `executeRunLike` helper that validates prompt ID, data
|
||||
package path, and output path, executes args, and returns a shared internal
|
||||
result struct.
|
||||
- Convert the shared result into `RunResult` or `StructuredRunResult` at the
|
||||
public method boundary so JSON artifact shapes remain unchanged.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Existing Scriptorium runner tests should remain behavior-preserving.
|
||||
- Add or keep table coverage that `Run` and `StructuredRun` produce the same
|
||||
argv, output-path field, truncation fields, and nonzero exit behavior.
|
||||
|
||||
Risk level: low. Adapter boundary is already narrow and well tested.
|
||||
|
||||
## Medium-confidence opportunities
|
||||
|
||||
### 1. Reduce repeated state save boilerplate without hiding artifact semantics
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/state/filesystem.go`
|
||||
- `internal/state/filesystem_test.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- Most save methods call `Paths`, select one path, write JSON or bytes
|
||||
atomically, and return the selected path.
|
||||
- Examples include `SaveModuleSnapshot`, `SavePreflight`,
|
||||
`SaveDistributorNotification`, `SaveGeneratedTextRaw`,
|
||||
`SaveGeneratedTextResult`, `SaveGeneratedText`, and `SaveRenderContext`.
|
||||
|
||||
Semantic differences that may be intentional:
|
||||
|
||||
- Some methods validate payloads before writing.
|
||||
- `SaveDataPackage` delegates to `promptinput.Save`.
|
||||
- `SaveMetadata` intentionally writes to the explicit metadata path stored on
|
||||
`state.Metadata`.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- Adding a new artifact type requires repeating the same path/write pattern.
|
||||
- Error context could drift if some methods wrap paths and others do not.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Consider small private helpers such as `saveJSONArtifact(resolved, pathFn,
|
||||
value)` and `saveBytesArtifact(resolved, pathFn, data)` only if a new artifact
|
||||
type is added or the save methods change again.
|
||||
- Do not create a generic manifest or artifact framework in this cleanup pass.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Keep path tests and artifact save/load tests.
|
||||
- If helpers are introduced, add one focused test that a representative JSON
|
||||
artifact and byte artifact still write to the same managed paths.
|
||||
|
||||
Risk level: low.
|
||||
|
||||
### 2. Extract package-local CLI integration test fixture helpers
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/cli/root_test.go`
|
||||
- `internal/app/app_test.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- CLI integration tests carry large fake Scriptorium shell scripts with repeated
|
||||
prompt-specific JSON cases for today, tomorrow, daily, and hourly.
|
||||
- Config string construction and test Weather API setup are repeated across
|
||||
groups of tests.
|
||||
|
||||
Semantic differences that may be intentional:
|
||||
|
||||
- Some fake scripts exercise markdown-output mode, some structured-output mode,
|
||||
and some failure behavior.
|
||||
- These tests are integration-style and intentionally explicit.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- As report types and prompt schemas grow, the shell fixture blocks become a
|
||||
brittle place to update prompt IDs, generated text schema shape, and expected
|
||||
report behavior.
|
||||
- The large test file makes failures harder to localize.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Add package-local helper builders for fake Scriptorium behavior, for example a
|
||||
map from prompt ID to JSON response plus a failure map for specific prompts.
|
||||
- Keep helpers local to `internal/cli` and `internal/app`; do not introduce a
|
||||
cross-package test framework.
|
||||
- Keep a few explicit end-to-end tests that prove real command wiring still
|
||||
works.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- No new behavior tests are required before helper extraction.
|
||||
- After extraction, run `go test ./internal/cli ./internal/app` and compare key
|
||||
assertions around generated-text artifacts, CLI output, and batch summaries.
|
||||
|
||||
Risk level: low.
|
||||
|
||||
### 3. Clarify config-to-briefing registry boundary
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/config/reports.go`
|
||||
- `internal/briefing/modules.go`
|
||||
- `internal/module/module.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- Not primarily duplication. This is a boundary concern: `internal/config`
|
||||
imports `internal/briefing` to initialize the default module registry and
|
||||
validate report module composition.
|
||||
|
||||
Semantic differences that may be intentional:
|
||||
|
||||
- The briefing registry is currently the implemented source of module builders,
|
||||
supported reports, options, missing-data behavior, and prompt exporters.
|
||||
- Config validation needs option schemas and composition rules, so using the
|
||||
registry is pragmatic.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- `internal/config` now depends on the module-builder package, not only on
|
||||
stable module option metadata. That is workable today, but it makes the
|
||||
config package pull in more of the report-building layer than it strictly
|
||||
needs.
|
||||
- If modules become more numerous or more expensive to initialize, config
|
||||
validation may become harder to keep side-effect free.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Do not split this immediately.
|
||||
- If module catalog complexity grows, consider separating build-free module
|
||||
definition metadata from module builders. The metadata/catalog can validate
|
||||
config options and composition; `internal/briefing` can attach builders and
|
||||
prompt exporters.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Keep config tests that prove report/module overrides fail during config load.
|
||||
- If a build-free catalog is introduced later, add tests proving config
|
||||
validation and runtime module build registry accept the same module IDs,
|
||||
options, supported reports, and missing-data policies.
|
||||
|
||||
Risk level: medium if deferred too long, low today.
|
||||
|
||||
### 4. Keep Weather API source fan-out explicit, but watch source metadata drift
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/adapters/weatherapi/client.go`
|
||||
- `internal/weatherdata/bundle.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
|
||||
- Each source fetch follows the same rough pattern: declare target, call
|
||||
`fetchDecodedSource`, attach source timestamps, assign bundle field, and add
|
||||
source provenance.
|
||||
|
||||
Semantic differences that may be intentional:
|
||||
|
||||
- Alerts intentionally special-case `data:null`.
|
||||
- Hourly is required and validates non-empty periods.
|
||||
- Weather story omits units.
|
||||
- SPC outlooks use endpoint constants and different issued/updated rules.
|
||||
|
||||
Why it matters:
|
||||
|
||||
- The current code is readable and not over-abstracted. The risk is future
|
||||
source additions repeating timestamp/hash/missing-policy decisions by hand.
|
||||
|
||||
Recommended refactor:
|
||||
|
||||
- Do not introduce a generic source ingestion framework now.
|
||||
- When the next source is added, consider adding small source-spec helpers for
|
||||
only the shared provenance fields that have identical semantics.
|
||||
|
||||
Suggested tests:
|
||||
|
||||
- Maintain focused adapter tests for each source's endpoint, query parameters,
|
||||
missing-source policy, and source metadata.
|
||||
|
||||
Risk level: low.
|
||||
|
||||
## Boundary and responsibility concerns
|
||||
|
||||
- `internal/config` depending on `internal/briefing` for module registry
|
||||
validation is the clearest boundary ambiguity. It is currently pragmatic, but
|
||||
a future build-free module catalog would fit the architecture better if
|
||||
module option/config complexity grows.
|
||||
- `internal/app` owns distributor notification request construction, including
|
||||
template values derived from report metadata. That is acceptable because
|
||||
orchestration owns the report result and managed report path, while
|
||||
distributor package types remain inside `internal/adapters/distributor`.
|
||||
- `internal/generatedtext` currently owns render-context assembly from module
|
||||
snapshots. That is a reasonable home, but the day-style report duplication
|
||||
should be reduced inside that package rather than moved into templates,
|
||||
`internal/app`, or `internal/reporttemplate`.
|
||||
- `internal/reporttemplate` correctly owns embedded template/schema lookup and
|
||||
rendering only. It should not absorb generated-text validation or report
|
||||
module policy.
|
||||
|
||||
## Path, key, and naming construction review
|
||||
|
||||
Local workspace paths are centralized in `internal/state.FilesystemStore.Paths`.
|
||||
This is a strong point: module snapshots, metadata, data packages, preflight
|
||||
artifacts, notification artifacts, generated-text artifacts, render contexts,
|
||||
and managed reports all derive from a single path function.
|
||||
|
||||
Distributor bundle path rendering is centralized in `internal/config` through
|
||||
`RenderDistributorReportPaths` and related template renderers. `internal/app`
|
||||
only assembles template values and maps the managed report path to rendered
|
||||
bundle paths. This is appropriate.
|
||||
|
||||
Report identity, artifact groups, batch output names, prompt IDs, template IDs,
|
||||
and schema IDs are declared in `internal/report` definitions. Current report
|
||||
definition files are now one report per file for active report types, which is
|
||||
easy to navigate.
|
||||
|
||||
Areas needing cleanup:
|
||||
|
||||
- The repeated state save methods can be lightly helperized later, but path
|
||||
construction itself is centralized enough.
|
||||
- Day-style template paths/schema IDs are declared in both `internal/report`
|
||||
definitions and `internal/generatedtext`/`internal/reporttemplate` catalogs.
|
||||
This is acceptable because those packages own different parts of the
|
||||
contract, but tests should continue asserting catalog compatibility.
|
||||
|
||||
## Resolution and catalog review
|
||||
|
||||
Report resolution is consistent:
|
||||
|
||||
- CLI command names resolve through `internal/report.IDForCommandName`.
|
||||
- Config report keys resolve through `internal/report.IDForConfigKey`.
|
||||
- Batches resolve through `internal/report.BatchReports`.
|
||||
- Report definitions own valid-period resolvers and module composition.
|
||||
|
||||
Module resolution is consistent:
|
||||
|
||||
- Report definitions and config overrides use `module.ConfigItem`.
|
||||
- Config module options are normalized and composition is validated before app
|
||||
use.
|
||||
- Runtime module execution uses `internal/briefing.ModuleRegistry`.
|
||||
- Prompt-input category mapping is centralized in `internal/promptinput`.
|
||||
|
||||
Generated-text resolution is mostly consistent:
|
||||
|
||||
- `internal/report` declares generation mode, template ID, and generated-text
|
||||
schema ID.
|
||||
- `internal/generatedtext.LookupDefinition` checks schema/template support and
|
||||
pairing.
|
||||
- `internal/reporttemplate` owns embedded asset lookup.
|
||||
|
||||
Recommended centralization:
|
||||
|
||||
- Consolidate day-style generated-text validation and common render-context
|
||||
extraction inside `internal/generatedtext`.
|
||||
- Keep separate report-specific public IDs, schemas, prompt assets, and
|
||||
template files.
|
||||
|
||||
## Config and command-loading review
|
||||
|
||||
Configuration loading is centralized in `internal/config.Load` with the
|
||||
documented precedence: CLI overrides, config file, built-in defaults.
|
||||
Environment secrets load through `secrets.directory` after config file parsing
|
||||
and CLI overrides, before validation completes. CLI commands consistently call
|
||||
`config.Load` rather than independently applying defaults.
|
||||
|
||||
Intentional differences:
|
||||
|
||||
- `generate` accepts `--out`; `run` accepts `--out-dir`; inspect commands do
|
||||
not accept weather overrides.
|
||||
- `generate daily` requires `--date`; `generate today` accepts optional
|
||||
`--date`; storm requires `--start`/`--end`.
|
||||
- Distributor has no CLI flags and is config-only.
|
||||
|
||||
Likely accidental or cleanup-worthy differences:
|
||||
|
||||
- Report-module config traversal is duplicated across normalization,
|
||||
validation, and override extraction. This is the main config cleanup target.
|
||||
|
||||
## State, manifest, or progress handling review
|
||||
|
||||
The application has durable workspace state but no manifest/resume engine.
|
||||
That is appropriate for the current scope.
|
||||
|
||||
State handling is consistent in the implemented flow:
|
||||
|
||||
- managed paths are computed by `state.FilesystemStore.Paths`;
|
||||
- module snapshots, data packages, preflight output, generated-text artifacts,
|
||||
render context, metadata, and notification debug artifacts are persisted
|
||||
under managed paths;
|
||||
- metadata links the relevant artifact paths;
|
||||
- prior snapshot lookup uses stored metadata and report compatibility policy;
|
||||
- inspection reads metadata and linked artifacts rather than refetching data.
|
||||
|
||||
Potential drift to watch:
|
||||
|
||||
- Metadata is saved several times during generated-text-template reports as
|
||||
artifacts become available. This is operationally useful for diagnosis, but
|
||||
future artifact additions should preserve the same pattern deliberately.
|
||||
- There is no retry/resume manifest. Do not add one unless operational
|
||||
requirements become concrete.
|
||||
|
||||
## Refactors to avoid
|
||||
|
||||
- Do not introduce a generic workflow engine for generation. The current
|
||||
explicit app orchestration is readable and well covered.
|
||||
- Do not migrate to Cobra or redesign the CLI. The standard-library CLI remains
|
||||
adequate.
|
||||
- Do not introduce a plugin architecture for reports, modules, or adapters.
|
||||
- Do not create per-module or per-report Go packages. Recent file-level
|
||||
separation is sufficient.
|
||||
- Do not build a broad Weather API source ingestion framework yet. Keep source
|
||||
fetches explicit until repeated source semantics become materially expensive.
|
||||
- Do not replace module snapshots with a manifest system in this cleanup pass.
|
||||
- Do not create a global test helper package. Use package-local helpers where
|
||||
test setup is noisy.
|
||||
- Do not consolidate Daily, Today, and Tomorrow into one public report type.
|
||||
Their public identities and templates are intentionally independent.
|
||||
|
||||
## Recommended implementation sequence
|
||||
|
||||
1. **GeneratedText day-report helper cleanup**
|
||||
- Goal: reduce Daily/Today/Tomorrow validation and render-context
|
||||
duplication while preserving separate public report types and template
|
||||
files.
|
||||
- Files: `internal/generatedtext/*.go`, generatedtext tests.
|
||||
- Validation: `go test ./internal/generatedtext`.
|
||||
|
||||
2. **Day-style template duplication cleanup**
|
||||
- Goal: reduce repeated Daypart Forecast and Precipitation Timing template
|
||||
logic without eliminating per-report templates.
|
||||
- Files: `internal/reporttemplate/templates/*.md.tmpl`,
|
||||
`internal/reporttemplate/reporttemplate.go` if named partial parsing is
|
||||
used, template tests, `docs/templates.md`.
|
||||
- Validation: `go test ./internal/reporttemplate ./internal/generatedtext`.
|
||||
|
||||
3. **Report-module config traversal cleanup**
|
||||
- Goal: use one canonical traversal for report override normalization,
|
||||
validation, and extraction.
|
||||
- Files: `internal/config/reports.go`, config tests.
|
||||
- Validation: `go test ./internal/config ./internal/app`.
|
||||
|
||||
4. **Scriptorium run-result helper cleanup**
|
||||
- Goal: share run/structured-run execution result construction while keeping
|
||||
exported result structs stable.
|
||||
- Files: `internal/adapters/scriptorium/runner.go`,
|
||||
`internal/adapters/scriptorium/runner_test.go`.
|
||||
- Validation: `go test ./internal/adapters/scriptorium`.
|
||||
|
||||
5. **Package-local CLI/app test fixture cleanup**
|
||||
- Goal: reduce repeated fake Scriptorium/config setup.
|
||||
- Files: `internal/cli/root_test.go`, optionally `internal/app/app_test.go`.
|
||||
- Validation: `go test ./internal/cli ./internal/app`.
|
||||
|
||||
6. **Optional state save helper cleanup**
|
||||
- Goal: reduce repeated `Paths` plus atomic write boilerplate only if the
|
||||
prior steps touch state tests or a new artifact type is being added.
|
||||
- Files: `internal/state/filesystem.go`, `internal/state/filesystem_test.go`.
|
||||
- Validation: `go test ./internal/state ./internal/app`.
|
||||
|
||||
7. **Final documentation and validation**
|
||||
- Goal: update implemented docs for any changed internal contracts, then run
|
||||
full validation.
|
||||
- Files: relevant `docs/internal/*`, `docs/templates.md`,
|
||||
`docs/policy/development.md` only if workflow changes.
|
||||
- Validation: `go test ./...`, `go run ./cmd/weatherreporter --help`,
|
||||
`git diff --check`.
|
||||
|
||||
## Test strategy
|
||||
|
||||
Tests to add before or during cleanup:
|
||||
|
||||
- `internal/generatedtext`: table tests proving Daily/Today/Tomorrow share the
|
||||
same generated-text required-field and unknown-field behavior.
|
||||
- `internal/generatedtext`: tests proving common day-style module extraction
|
||||
still returns report-specific planning modules.
|
||||
- `internal/reporttemplate`: tests for shared daypart and precipitation
|
||||
rendering behavior after template cleanup.
|
||||
- `internal/config`: tests proving `Load`, `Validate`, and
|
||||
`ReportModuleOverrides` share duplicate alias and invalid module behavior.
|
||||
- `internal/adapters/scriptorium`: tests proving `Run` and `StructuredRun`
|
||||
preserve argv, output path, captured output, truncation flags, and nonzero
|
||||
exit behavior.
|
||||
- `internal/cli` and `internal/app`: keep workflow tests for generated-text
|
||||
artifacts, data-package output, notification behavior, and batch summaries.
|
||||
|
||||
Validation commands for cleanup work:
|
||||
|
||||
```sh
|
||||
go test ./internal/generatedtext ./internal/reporttemplate
|
||||
go test ./internal/config ./internal/app
|
||||
go test ./internal/adapters/scriptorium
|
||||
go test ./internal/cli ./internal/state
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Lightweight audit validation performed for this report:
|
||||
|
||||
```sh
|
||||
go list ./...
|
||||
```
|
||||
|
||||
## Appendix: findings not worth acting on
|
||||
|
||||
- **Weather API source fetch functions look similar but should remain explicit
|
||||
for now.** Each source has different required/optional behavior, endpoint
|
||||
query options, timestamp rules, and null-data semantics. A generic framework
|
||||
would make the current adapter harder to read.
|
||||
- **Report definition files intentionally repeat field names.** Each report
|
||||
definition should remain explicit about prompt ID, template ID, artifact
|
||||
group, batch output name, compatibility, and modules.
|
||||
- **CLI flag parsing uses repeated `flag.FlagSet` setup.** The current parser is
|
||||
small and clear. Additional abstraction would not reduce much risk beyond the
|
||||
existing `addCommonFlags` helper.
|
||||
- **`internal/app.GenerateReport` is long but linear.** It is the main
|
||||
orchestration function and currently reads in the same order as the workflow.
|
||||
Splitting it aggressively would risk hiding stage ordering. Prefer extracting
|
||||
only small repeated mechanics.
|
||||
- **Generated Markdown templates are necessarily editable assets.** Do not
|
||||
replace template wording with Go string builders. Cleanup should preserve the
|
||||
user's ability to edit report layout and prose structure in template files.
|
||||
481
docs/roadmap/cleanup.md
Normal file
481
docs/roadmap/cleanup.md
Normal file
@@ -0,0 +1,481 @@
|
||||
# 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 each stage in order.
|
||||
|
||||
The cleanup sequence is intentionally narrow. It should reduce duplication and
|
||||
clarify package responsibilities before the next major release without changing
|
||||
public CLI syntax, user configuration, report identities, managed artifact
|
||||
paths, or generated report behavior.
|
||||
|
||||
This file may describe planned work because it lives under `docs/roadmap/`.
|
||||
|
||||
## Cleanup Principles
|
||||
|
||||
- Preserve public behavior unless a stage explicitly says otherwise.
|
||||
- Prefer small behavior-preserving refactors over broad rewrites.
|
||||
- Keep domain policy in the package that owns the relevant contract.
|
||||
- Keep external system details behind adapter boundaries.
|
||||
- Keep report, module, template, schema, path, and artifact identity explicit.
|
||||
- Add tests before or during cleanup where they protect public behavior or
|
||||
important internal invariants.
|
||||
- Update implemented documentation only after code behavior exists.
|
||||
- Do not use cleanup as an opportunity to introduce new features.
|
||||
|
||||
## Locked Decisions
|
||||
|
||||
- Daily, Today, and Tomorrow remain separate report IDs, prompt IDs, schemas,
|
||||
templates, and public generated-text types.
|
||||
- Cleanup may reduce shared internal day-style plumbing, but must not merge the
|
||||
public report types.
|
||||
- Daily, Today, and Tomorrow keep separate top-level template files.
|
||||
- Named template partials may be introduced for repeated Daypart Forecast and
|
||||
Precipitation Timing blocks.
|
||||
- Configuration ownership remains in `internal/config`; config parsing and
|
||||
validation should not move into `internal/app` or `internal/briefing`.
|
||||
- Scriptorium and distributor dependency details must remain behind their
|
||||
adapter packages.
|
||||
- Existing public CLI syntax, config fields, report IDs, prompt IDs, template
|
||||
IDs, schema IDs, workspace paths, distributor paths, and generated report
|
||||
paths remain stable.
|
||||
- Do not introduce Cobra, a workflow engine, a plugin system,
|
||||
manifest/resume/progress infrastructure, per-module packages, per-report
|
||||
packages, or a global test helper package.
|
||||
|
||||
## Stage 1: Day-Style GeneratedText Validation Helpers
|
||||
|
||||
### Goal
|
||||
|
||||
Remove duplicated Daily/Today/Tomorrow generated-text validation while
|
||||
preserving public types and JSON schema behavior.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add an unexported shared helper in `internal/generatedtext` for the common
|
||||
day-style shape: `summary`, `forecast_discussion`, optional
|
||||
`precipitation_timing`, and optional `confidence`.
|
||||
- Keep exported `Daily`, `Today`, and `Tomorrow` structs.
|
||||
- Keep exported `ValidateDaily`, `ValidateToday`, and `ValidateTomorrow`
|
||||
functions.
|
||||
- Preserve normalized JSON output shape and unknown-field rejection.
|
||||
- Preserve current error messages except for the expected report-name
|
||||
substitution.
|
||||
- Do not change embedded generated-text schemas in this stage except as needed
|
||||
to keep tests aligned with existing behavior.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Daily, Today, and Tomorrow validation use one shared internal validation path
|
||||
for common trim, required-field, optional-field, and normalization behavior.
|
||||
- Public generated-text structs and function names remain unchanged.
|
||||
- Existing callers do not need to change.
|
||||
- Existing schema behavior remains unchanged.
|
||||
|
||||
### Tests
|
||||
|
||||
- Add table coverage proving Daily, Today, and Tomorrow share:
|
||||
- required `summary` behavior;
|
||||
- required non-empty `forecast_discussion` behavior;
|
||||
- trim behavior;
|
||||
- optional-field omission behavior;
|
||||
- normalized JSON output behavior;
|
||||
- unknown-field rejection.
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/generatedtext
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 2: Day-Style Render Context And Template Shared Blocks
|
||||
|
||||
### Goal
|
||||
|
||||
Reduce repeated Daily/Today/Tomorrow render-context and Markdown template logic
|
||||
without merging the reports.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add shared unexported helpers for common day-style report context fields such
|
||||
as forecast date, forecast date label, generated timestamp label, valid
|
||||
period, timezone, collected facts, and derived facts.
|
||||
- Add a common module snapshot extraction helper for shared day-style modules.
|
||||
- Keep report-specific planning modules separate:
|
||||
- Daily uses `DailyPlanning`;
|
||||
- Today uses `TodayPlanning`;
|
||||
- Tomorrow uses `TomorrowPlanning`.
|
||||
- Keep report-specific daypart wrapper types if tests or templates benefit from
|
||||
explicit names.
|
||||
- Add named template partial support in `internal/reporttemplate` for repeated
|
||||
Daypart Forecast and Precipitation Timing blocks.
|
||||
- Keep `daily.md.tmpl`, `today.md.tmpl`, and `tomorrow.md.tmpl` as separate
|
||||
top-level templates that opt into shared partials.
|
||||
- Preserve current rendered Markdown behavior, including Today's omission of
|
||||
elapsed or missing daypart lines.
|
||||
- Do not replace editable Markdown templates with Go string builders.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Common day-style render-context setup is shared internally.
|
||||
- Daily, Today, and Tomorrow still expose their own render context types.
|
||||
- Shared template partials reduce repeated daypart and precipitation template
|
||||
logic.
|
||||
- Report templates remain separately editable.
|
||||
- Current rendered output remains stable except for whitespace changes that are
|
||||
covered by updated tests and intentionally accepted.
|
||||
|
||||
### Tests
|
||||
|
||||
- Update render-context tests to prove:
|
||||
- common module fields are populated for Daily, Today, and Tomorrow;
|
||||
- each report still exposes its correct planning module;
|
||||
- Today still omits missing/elapsed dayparts where current behavior expects
|
||||
omission.
|
||||
- Update reporttemplate tests to prove Daily, Today, and Tomorrow output remains
|
||||
behaviorally stable.
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/generatedtext ./internal/reporttemplate
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Likely one implementation prompt. If template partial parsing changes and
|
||||
render-context helper extraction become difficult to review together, split
|
||||
this into:
|
||||
|
||||
1. render-context helper cleanup;
|
||||
2. template partial cleanup.
|
||||
|
||||
## Stage 3: Report Module Config Traversal Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Make report-module config normalization, validation, and override extraction use
|
||||
one canonical traversal.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Refactor `internal/config/reports.go` around one unexported helper that:
|
||||
- resolves report config keys through `internal/report`;
|
||||
- detects duplicate aliases;
|
||||
- validates report IDs against the report registry;
|
||||
- normalizes module options when requested;
|
||||
- builds `module.ConfigItem` values;
|
||||
- validates module composition through the module registry.
|
||||
- Reuse that helper from:
|
||||
- `normalizeReportModules`;
|
||||
- `validateReportModules`;
|
||||
- `ReportModuleOverrides`.
|
||||
- Preserve current configuration precedence and YAML shape.
|
||||
- Preserve current option normalization behavior.
|
||||
- Preserve error context such as
|
||||
`reports.<key>.deterministic_modules[...]`.
|
||||
- Keep `internal/config` as the owner of config loading, normalization, and
|
||||
validation.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Report-module config traversal exists in one implementation path.
|
||||
- Load-time normalization, validation, and override extraction cannot drift on
|
||||
report-key or module composition policy.
|
||||
- Existing config files continue to load unchanged.
|
||||
- Existing config error messages remain materially equivalent and actionable.
|
||||
|
||||
### Tests
|
||||
|
||||
- Keep or update tests for:
|
||||
- unknown report keys;
|
||||
- duplicate report aliases;
|
||||
- unknown modules;
|
||||
- duplicate modules;
|
||||
- incompatible modules;
|
||||
- invalid module options;
|
||||
- valid module overrides.
|
||||
- Add coverage proving loaded config and manually constructed config fail
|
||||
consistently for the same invalid report/module cases.
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/config ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 4: Scriptorium Run Plumbing Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Share `Run` and `StructuredRun` execution mechanics while keeping the
|
||||
Scriptorium adapter API stable.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add an unexported run-like helper in `internal/adapters/scriptorium` that:
|
||||
- validates prompt ID;
|
||||
- validates data package path;
|
||||
- validates output path;
|
||||
- executes the resolved argv;
|
||||
- captures stdout and stderr;
|
||||
- records truncation flags;
|
||||
- records output path;
|
||||
- handles nonzero exit results.
|
||||
- Keep exported `RunRequest`, `StructuredRunRequest`, `RunResult`, and
|
||||
`StructuredRunResult`.
|
||||
- Keep `StructuredRun` using the same argv shape as `Run`; do not add schema or
|
||||
format flags.
|
||||
- Preserve argv order.
|
||||
- Preserve stdout/stderr capture and truncation fields.
|
||||
- Preserve output-path fields.
|
||||
- Preserve existing nonzero-exit error wording as closely as possible.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- `Run` and `StructuredRun` share validation and result construction mechanics.
|
||||
- Public adapter request/result types remain stable.
|
||||
- Existing app-layer Scriptorium calls do not need behavior changes.
|
||||
- Existing Scriptorium tests still pass with minimal expected-output updates.
|
||||
|
||||
### Tests
|
||||
|
||||
- Add or update parity tests proving `Run` and `StructuredRun` preserve:
|
||||
- argv construction;
|
||||
- output path;
|
||||
- captured stdout/stderr;
|
||||
- truncation flags;
|
||||
- nonzero exit result and error behavior.
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/adapters/scriptorium
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 5: CLI Test Fixture Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Reduce noisy repeated CLI integration test setup without creating a
|
||||
cross-package test framework.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add package-local helpers in `internal/cli` tests for repeated:
|
||||
- fake Scriptorium script setup;
|
||||
- generated-text JSON responses;
|
||||
- config file writing;
|
||||
- Weather API test server setup;
|
||||
- artifact path or glob assertions.
|
||||
- Keep a few explicit CLI workflow tests readable end-to-end.
|
||||
- Do not add `internal/testutil` or another global test helper package.
|
||||
- Do not weaken assertions while deduplicating setup.
|
||||
- Do not change production CLI behavior in this stage.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Test setup repetition is reduced in the largest CLI test file.
|
||||
- Test behavior and coverage remain equivalent.
|
||||
- Helpers are local to `internal/cli`.
|
||||
- No production code changes are required for this stage.
|
||||
|
||||
### Tests
|
||||
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/cli
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 6: App Test Fixture Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Reduce noisy repeated app orchestration test setup without creating a
|
||||
cross-package test framework.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add package-local helpers in `internal/app` tests where repetition is high and
|
||||
the helper improves readability.
|
||||
- Good candidates include repeated fake renderer setup, generated-text JSON
|
||||
responses, config file writing, Weather API test server setup, distributor
|
||||
notifier setup, recording store setup, and artifact assertions.
|
||||
- Keep key workflow tests readable end-to-end so generation ordering remains
|
||||
clear.
|
||||
- Do not add `internal/testutil` or another global test helper package.
|
||||
- Do not weaken assertions while deduplicating setup.
|
||||
- Do not change production app behavior in this stage.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Test setup repetition is reduced in the largest app test file.
|
||||
- Test behavior and coverage remain equivalent.
|
||||
- Helpers are local to `internal/app`.
|
||||
- No production code changes are required for this stage.
|
||||
|
||||
### Tests
|
||||
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 7: State Artifact Save Helper Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Reduce repeated filesystem artifact write boilerplate while keeping artifact
|
||||
semantics visible.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Add small private helpers in `internal/state/filesystem.go` for resolved JSON
|
||||
and byte artifact writes.
|
||||
- Keep artifact-specific validation in public save methods before calling any
|
||||
helper.
|
||||
- Keep each public save method explicit about which artifact path it writes.
|
||||
- Leave `SaveDataPackage` with its current special behavior unless a helper
|
||||
cleanly preserves `promptinput.Save`.
|
||||
- Leave `SaveMetadata` with its current explicit metadata-path validation and
|
||||
write behavior unless a helper cleanly preserves it.
|
||||
- Do not introduce a manifest, artifact registry, resume system, or broad
|
||||
artifact framework.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Repeated `Paths` plus atomic write mechanics are reduced where the semantics
|
||||
are identical.
|
||||
- Artifact-specific validation and path choice remain easy to see.
|
||||
- Managed artifact paths do not change.
|
||||
- Metadata JSON shape does not change.
|
||||
|
||||
### Tests
|
||||
|
||||
- Keep or update state save tests and metadata round-trip tests.
|
||||
- Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/state ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 8: Documentation And Final Validation
|
||||
|
||||
### Goal
|
||||
|
||||
Align implemented documentation only where cleanup changes internal contracts or
|
||||
template editing guidance.
|
||||
|
||||
### Implementation Guidance
|
||||
|
||||
- Update `docs/templates.md` if named template partial support changes how
|
||||
report templates should be edited.
|
||||
- Update relevant `docs/internal/*` files only for implemented internal
|
||||
contract changes.
|
||||
- Update `docs/policy/development.md` only if contributor workflow guidance
|
||||
changes.
|
||||
- Keep unimplemented or deferred cleanup ideas only under `docs/roadmap/`.
|
||||
- Do not document future refactors as implemented behavior.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Non-roadmap docs describe only implemented behavior.
|
||||
- Template-editing guidance matches the final template partial structure, if
|
||||
partials were added.
|
||||
- Internal docs remain accurate for generated text, templates, config, state,
|
||||
and adapters touched by cleanup.
|
||||
|
||||
### Validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/generatedtext ./internal/reporttemplate
|
||||
go test ./internal/config ./internal/app
|
||||
go test ./internal/adapters/scriptorium
|
||||
go test ./internal/cli ./internal/state
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Deferred Refactors
|
||||
|
||||
The following refactors are out of scope for this cleanup sequence:
|
||||
|
||||
- Generic workflow engine.
|
||||
- Cobra migration or CLI redesign.
|
||||
- Plugin architecture.
|
||||
- Per-module or per-report packages.
|
||||
- Broad Weather API source-ingestion framework.
|
||||
- Manifest, resume, or progress system.
|
||||
- Global test helper package.
|
||||
- Consolidating Daily, Today, and Tomorrow into one public report type.
|
||||
- Replacing editable Markdown templates with Go string builders.
|
||||
- Build-free module catalog split.
|
||||
|
||||
The build-free module catalog split may be revisited later if
|
||||
config/module-boundary complexity grows enough to justify separating module
|
||||
metadata from module builders.
|
||||
|
||||
## Global Validation Checklist
|
||||
|
||||
Run these checks after completing the full cleanup sequence:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Also run focused checks after the relevant stages:
|
||||
|
||||
```sh
|
||||
go test ./internal/generatedtext
|
||||
go test ./internal/generatedtext ./internal/reporttemplate
|
||||
go test ./internal/config ./internal/app
|
||||
go test ./internal/adapters/scriptorium
|
||||
go test ./internal/cli
|
||||
go test ./internal/app
|
||||
go test ./internal/state ./internal/app
|
||||
```
|
||||
|
||||
Manual review checklist:
|
||||
|
||||
- Public CLI syntax is unchanged.
|
||||
- Public config fields and defaults are unchanged.
|
||||
- Report IDs, prompt IDs, template IDs, and schema IDs are unchanged.
|
||||
- Managed workspace artifact paths are unchanged.
|
||||
- Distributor bundle paths and notification behavior are unchanged.
|
||||
- Generated report Markdown behavior is unchanged except for intentional,
|
||||
test-covered whitespace differences.
|
||||
- Scriptorium argv construction is unchanged.
|
||||
- Non-roadmap docs do not describe unimplemented cleanup work.
|
||||
@@ -1,348 +0,0 @@
|
||||
# Daily Report Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
This roadmap defines the target state and policy choices for replacing the
|
||||
existing `daily` report with a new generated-text-template `daily` report.
|
||||
|
||||
The new report is not implemented yet. Current report behavior remains
|
||||
documented outside `docs/roadmap/`.
|
||||
|
||||
## Intent
|
||||
|
||||
Daily should be an independent generated-text-template report for a user-chosen
|
||||
local civil day. Its rendered Markdown should initially match the Tomorrow
|
||||
Report format exactly, but its identity, prompt, template, schema, valid-period
|
||||
resolver, config key, and planning module should be separate from Tomorrow from
|
||||
the start.
|
||||
|
||||
The important distinction is date selection:
|
||||
|
||||
- `tomorrow` always targets the next local civil day.
|
||||
- `daily` targets the local civil day explicitly supplied by the user with
|
||||
`--date YYYY-MM-DD`.
|
||||
|
||||
This is a clean breaking replacement of the current Daily implementation:
|
||||
|
||||
- The existing direct-Markdown `daily` report is removed.
|
||||
- The legacy `daily_today` report ID is removed from active report definitions.
|
||||
- `weatherreporter generate daily` remains the public command name, but it now
|
||||
runs the new generated-text-template Daily report.
|
||||
- `weatherreporter generate daily` requires `--date YYYY-MM-DD`.
|
||||
- Existing historical `daily_today` workspace artifacts do not need migration.
|
||||
|
||||
## Locked Decisions
|
||||
|
||||
- New report ID: `daily`.
|
||||
- Public command: `generate daily`.
|
||||
- `generate daily` must require `--date YYYY-MM-DD`.
|
||||
- The provided date is interpreted as a civil date in the effective report
|
||||
timezone.
|
||||
- The valid period is the selected local civil day, `[00:00, next 00:00)`.
|
||||
- Daily should not be an alias for Today or Tomorrow.
|
||||
- Today remains the current-day scheduled morning product.
|
||||
- Tomorrow remains the next-day scheduled evening product.
|
||||
- Daily is manually targeted by date and is not added to morning or evening
|
||||
batch membership in this roadmap.
|
||||
- Daily must have its own template, generated-text schema, prompt asset,
|
||||
render-context type, and planning module.
|
||||
- Daily may share private helper functions with Tomorrow where mechanics are
|
||||
identical, but it must not expose Tomorrow-specific public types or stanzas.
|
||||
- The initial Daily output format should match Tomorrow's rendered Markdown
|
||||
format.
|
||||
|
||||
## Target Report Shape
|
||||
|
||||
Daily should render the same Markdown structure as Tomorrow:
|
||||
|
||||
```markdown
|
||||
# Monday's Weather
|
||||
|
||||
**Forecast date:** Monday, June 15, 2026
|
||||
**Updated:** Sunday, June 14, 2026 at 9:14 AM
|
||||
|
||||
<GeneratedText summary>
|
||||
|
||||
## Daypart Forecast
|
||||
|
||||
- **Morning:** <deterministic daypart line>
|
||||
- **Midday:** <deterministic daypart line>
|
||||
- **Afternoon:** <deterministic daypart line>
|
||||
- **Evening:** <deterministic daypart line>
|
||||
|
||||
## Precipitation Timing
|
||||
|
||||
- **1:00 PM** to **5:00 PM**: Precipitation is expected during this period.
|
||||
The peak precipitation chance is 59% at 2:00 PM.
|
||||
- <optional GeneratedText precipitation_timing>
|
||||
|
||||
## Forecast Discussion
|
||||
|
||||
<GeneratedText forecast_discussion paragraphs>
|
||||
```
|
||||
|
||||
The precipitation section should render only when precipitation windows exist
|
||||
for the selected valid period.
|
||||
|
||||
The title should follow Tomorrow's day-name style, for example:
|
||||
|
||||
- `Monday's Weather`
|
||||
- `Tuesday's Weather`
|
||||
- `Sunday's Weather`
|
||||
|
||||
## Report Identity
|
||||
|
||||
Replace the existing active Daily report with:
|
||||
|
||||
- report ID: `daily`
|
||||
- public generate command: `daily`
|
||||
- prompt ID: `weather.daily_generated_text`
|
||||
- generation mode: `generated_text_template`
|
||||
- template ID: `daily`
|
||||
- generated-text schema ID: `daily`
|
||||
- artifact group: `daily`
|
||||
- batch output name: `daily.md`
|
||||
- prior compatibility: Daily only
|
||||
- comparison strategy: same valid local date
|
||||
- valid period: selected local civil day in the effective report timezone,
|
||||
`[00:00, next 00:00)`
|
||||
|
||||
Remove the legacy active report identity:
|
||||
|
||||
- remove active report ID `daily_today`
|
||||
- remove prompt ID `weather.daily_report` from the current Daily path
|
||||
- remove direct-Markdown generation mode from the Daily report definition
|
||||
- remove `daily_today` config-key support unless a separate migration roadmap
|
||||
explicitly reintroduces it
|
||||
|
||||
Historical artifacts with `daily_today` metadata may remain on disk. Do not
|
||||
migrate or rewrite old workspace files in this feature.
|
||||
|
||||
## CLI Behavior
|
||||
|
||||
`weatherreporter generate daily` should require:
|
||||
|
||||
```sh
|
||||
weatherreporter generate daily --date YYYY-MM-DD
|
||||
```
|
||||
|
||||
Rules:
|
||||
|
||||
- `--date` is required for `generate daily`.
|
||||
- `--date` accepts only `YYYY-MM-DD`.
|
||||
- The date is interpreted in the effective report timezone after config and
|
||||
`--tz` overrides are applied.
|
||||
- Omitting `--date` is an error.
|
||||
- A malformed date is an error.
|
||||
- The command should continue supporting the existing global generation flags:
|
||||
`--config`, `--units`, `--tz`, and `--out`.
|
||||
- Do not default `daily` to today or tomorrow.
|
||||
|
||||
## Batch Behavior
|
||||
|
||||
Daily should not be added to scheduled batches in this roadmap.
|
||||
|
||||
Current intended scheduled behavior:
|
||||
|
||||
- Morning batch: `today`, `three_day`, and conditional `weekend`.
|
||||
- Evening batch: `tomorrow`.
|
||||
|
||||
Daily is a manually targeted report. A future roadmap may add scheduled Daily
|
||||
behavior if a concrete operational need appears.
|
||||
|
||||
## GeneratedText Contract
|
||||
|
||||
Daily should use the same structured prose shape as Tomorrow:
|
||||
|
||||
```json
|
||||
{
|
||||
"summary": "string",
|
||||
"forecast_discussion": ["string"],
|
||||
"precipitation_timing": "string",
|
||||
"confidence": "string"
|
||||
}
|
||||
```
|
||||
|
||||
Required:
|
||||
|
||||
- `summary`
|
||||
- `forecast_discussion`
|
||||
|
||||
Optional:
|
||||
|
||||
- `precipitation_timing`
|
||||
- `confidence`
|
||||
|
||||
Validation should match Tomorrow semantics:
|
||||
|
||||
- reject malformed JSON and unknown fields
|
||||
- reject trailing JSON values
|
||||
- trim `summary`, `precipitation_timing`, and `confidence`
|
||||
- trim each `forecast_discussion` paragraph
|
||||
- drop blank discussion paragraphs
|
||||
- require at least one nonblank discussion paragraph
|
||||
- return canonical normalized JSON with the same public field names
|
||||
|
||||
Add a dedicated prompt asset:
|
||||
|
||||
- `internal/reporttemplate/prompts/daily.generated_text.md`
|
||||
|
||||
Scriptorium registration remains out of band. Weatherreporter should invoke the
|
||||
Daily prompt by prompt ID and pass the data package as it does for other
|
||||
generated-text reports.
|
||||
|
||||
## Template Context
|
||||
|
||||
Add dedicated Daily types under `internal/generatedtext`, rather than reusing
|
||||
Tomorrow types directly:
|
||||
|
||||
```go
|
||||
type DailyRenderContext struct {
|
||||
Report DailyReportContext
|
||||
GeneratedText Daily
|
||||
Modules DailyTemplateModules
|
||||
Collected facts.CollectedFacts
|
||||
Derived facts.DerivedFacts
|
||||
}
|
||||
```
|
||||
|
||||
`DailyReportContext` should include:
|
||||
|
||||
- `Title`, for example `Monday's Weather`
|
||||
- `ForecastDate`
|
||||
- `ForecastDateLabel`, for example `Monday, June 15, 2026`
|
||||
- `ForecastDayName`, for example `Monday`
|
||||
- `GeneratedAt`
|
||||
- `GeneratedAtLabel`
|
||||
- `ValidPeriod`
|
||||
- `Timezone`
|
||||
|
||||
`DailyTemplateModules` should expose the same categories the Daily template
|
||||
needs:
|
||||
|
||||
- `Metadata`
|
||||
- `CurrentConditions`
|
||||
- `HourlyForecast`
|
||||
- `DerivedDailySummary`
|
||||
- `DerivedDaypartSummaries`
|
||||
- ordered daypart rows
|
||||
- `PrecipTiming`
|
||||
- `AlertDigest`
|
||||
- `SPCConvectiveOutlooks`
|
||||
- `AreaForecastDiscussion`
|
||||
- `SPCConvectiveDiscussion`
|
||||
- `WeatherStory`
|
||||
- `OutdoorWindows`
|
||||
- `DailyPlanning`
|
||||
|
||||
Daily may share private helper functions with Tomorrow render-context
|
||||
construction when the helper represents identical mechanics. Do not expose
|
||||
Tomorrow-specific types through the Daily template context.
|
||||
|
||||
## Module Composition
|
||||
|
||||
The default module composition should initially mirror Tomorrow where the same
|
||||
facts are useful for a dated daily report, with a Daily-specific planning
|
||||
module:
|
||||
|
||||
- `metadata`
|
||||
- `current_conditions`
|
||||
- `narrative_forecast`
|
||||
- `derived_daily_summary`
|
||||
- `derived_daypart_summaries`
|
||||
- `precip_timing`
|
||||
- `alert_digest`
|
||||
- `spc_convective_outlooks`
|
||||
- `area_forecast_discussion`
|
||||
- `spc_convective_discussion`
|
||||
- `weather_story`
|
||||
- `outdoor_windows`
|
||||
- `daily_planning`
|
||||
- `hourly_forecast`
|
||||
|
||||
The module order should match the intended data-package order unless tests show
|
||||
a stronger reason to mirror Tomorrow's exact current order.
|
||||
|
||||
## Daily Planning Module
|
||||
|
||||
Add a Daily-specific deterministic planning module:
|
||||
|
||||
- module ID: `daily_planning`
|
||||
- stanza name: `daily_planning`
|
||||
- options type: `DailyPlanningOptions`
|
||||
- output type: `DailyPlanningModule`
|
||||
- supported report: `daily`
|
||||
|
||||
The module should be initially equivalent to `TomorrowPlanning`, but independent
|
||||
from it:
|
||||
|
||||
- do not reuse the public `TomorrowPlanningModule` type
|
||||
- do not emit the `tomorrow_planning` stanza
|
||||
- do not use `module.TomorrowPlanning` in the Daily default composition
|
||||
|
||||
Recommended initial fields should match Tomorrow planning:
|
||||
|
||||
- `morning_readiness`
|
||||
- `commute_school_workday_concerns`
|
||||
- `overnight_change_watch`
|
||||
|
||||
Private helper functions may be shared with Tomorrow planning when the
|
||||
underlying logic is truly identical.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
The feature is complete when:
|
||||
|
||||
- `weatherreporter generate daily --date YYYY-MM-DD` runs the new Daily report.
|
||||
- `weatherreporter generate daily` without `--date` fails with an actionable
|
||||
error.
|
||||
- `daily` report metadata, RunID content, artifact paths, data-package paths,
|
||||
distributor template variables, generated-text assets, and rendered Markdown
|
||||
all use report ID `daily`.
|
||||
- The active report registry includes `daily` and does not include
|
||||
`daily_today`.
|
||||
- `reports.daily` is the implemented config override key for Daily.
|
||||
- `reports.daily_today` is rejected rather than treated as an alias.
|
||||
- Daily uses generated-text-template generation with prompt ID
|
||||
`weather.daily_generated_text`.
|
||||
- Daily uses dedicated schema, prompt, template, generated-text type,
|
||||
render-context type, and planning-module surfaces.
|
||||
- Daily rendered Markdown initially matches Tomorrow's report format.
|
||||
- Daily data packages include `daily_planning`, not `tomorrow_planning`.
|
||||
- Daily Recent Changes compare against prior Daily snapshots for the same valid
|
||||
local date.
|
||||
- Daily is not included in morning or evening scheduled batches.
|
||||
- Existing Today and Tomorrow report semantics remain unchanged.
|
||||
- Historical `daily_today` workspace artifacts are left untouched.
|
||||
- Non-roadmap documentation is updated after implementation to describe only
|
||||
implemented Daily behavior.
|
||||
|
||||
## Implementation Plan Reference
|
||||
|
||||
Use `docs/roadmap/implementation.md` for the staged implementation plan. This
|
||||
feature roadmap intentionally does not define implementation stages, file-by-file
|
||||
work packages, or validation commands so that implementing agents have a single
|
||||
sequencing authority.
|
||||
|
||||
## Ambiguities Addressed
|
||||
|
||||
- Replacement scope: new `daily` replaces and removes old active
|
||||
`daily_today`.
|
||||
- Date behavior: `--date` is required; no default date is used.
|
||||
- Output format: initial rendered Markdown matches Tomorrow.
|
||||
- Internal separation: Daily has its own template, schema, prompt, generated
|
||||
text type, render context, and planning module.
|
||||
- Batch behavior: Daily is not scheduled; Today remains the morning current-day
|
||||
scheduled product.
|
||||
- Historical artifacts: old `daily_today` workspace files are not migrated.
|
||||
|
||||
## Open Decisions
|
||||
|
||||
No open decisions remain that block implementation.
|
||||
|
||||
Future decisions that should not be resolved in this roadmap:
|
||||
|
||||
- Whether Daily should eventually support recurring scheduled generation.
|
||||
- Whether Daily should diverge from Tomorrow's template or planning logic.
|
||||
- Whether old `daily_today` workspace artifacts should ever receive a migration
|
||||
or inspection compatibility layer.
|
||||
@@ -1,361 +0,0 @@
|
||||
# Data Package Export Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
This roadmap defines the target state for cleaning up module fields exposed in
|
||||
YAML data packages. The goal is to keep report templates composable while making
|
||||
LLM prompt inputs concise, readable, and free of template-only helper fields.
|
||||
|
||||
This feature is implemented. Current data-package behavior is documented in
|
||||
`docs/internal/prompt-input.md`; the rich module and template boundary is
|
||||
documented in `docs/internal/module.md` and `docs/templates.md`.
|
||||
|
||||
## Problem
|
||||
|
||||
Module output structs currently serve two different consumers:
|
||||
|
||||
- deterministic report templates, which benefit from presentation helpers such
|
||||
as lower-case text, display labels, trend phrases, and hour labels;
|
||||
- Scriptorium data packages, which should expose the clearest useful weather
|
||||
facts to the LLM with minimal redundancy.
|
||||
|
||||
Those consumers now need different surfaces. Examples include:
|
||||
|
||||
- `current_conditions` exposes both `condition_text` and
|
||||
`condition_text_lower`, plus both abbreviated and long-form wind-direction
|
||||
fields.
|
||||
- `hourly_forecast.periods[]` exposes both `period_begins` and `hour_label`,
|
||||
and both `text_description` and `text_description_lower`.
|
||||
- `derived_daypart_summaries` exposes numerous temperature and condition phrase
|
||||
fields that are useful for deterministic template wording but noisy in the
|
||||
prompt data package.
|
||||
|
||||
The cleanup should not weaken template composability. Templates should still be
|
||||
able to use rich module values and helper fields.
|
||||
|
||||
The cleanup applies to every report that consumes these modules, including the
|
||||
generated-template `today`, `tomorrow`, and `daily` reports. The same
|
||||
`derived_daypart_summaries` prompt export should serve all three reports while
|
||||
their templates continue to use rich daypart helper fields.
|
||||
|
||||
## Intent
|
||||
|
||||
Data packages should be curated prompt inputs, not a raw dump of every field
|
||||
available to Go templates.
|
||||
|
||||
The intended architecture is:
|
||||
|
||||
- module builders produce rich internal/template module values;
|
||||
- each module may define a prompt-facing export value for data-package use;
|
||||
- prompt input construction serializes the prompt-facing export value;
|
||||
- template rendering continues to use the full rich module value.
|
||||
|
||||
The result should let `weatherreporter` optimize separately for:
|
||||
|
||||
- precise deterministic Markdown rendering;
|
||||
- compact, readable LLM input;
|
||||
- stable internal module contracts.
|
||||
|
||||
## Locked Decisions
|
||||
|
||||
- Do not make the existing module structs smaller solely to clean up data
|
||||
packages.
|
||||
- Do not use per-module string field allowlists as the primary mechanism.
|
||||
- Do not rely on reflection-heavy field filtering for nested module shapes.
|
||||
- Do not use `json:"-"` or `yaml:"-"` on rich template fields as the main
|
||||
boundary.
|
||||
- Keep rich module outputs available for template rendering, inspection, tests,
|
||||
and internal use.
|
||||
- Add an explicit prompt/data-package export layer for module outputs.
|
||||
- Simple modules may use default pass-through export behavior.
|
||||
- No compatibility aliases are needed for removed prompt-facing fields because
|
||||
the prompt schema is still pre-release.
|
||||
- Bump the data-package schema version when implementing this change.
|
||||
- Compute prompt export values during module snapshot construction and store the
|
||||
runtime-only prompt value on `module.Output` alongside the rich `Value`.
|
||||
- Do not persist prompt export values in module snapshot JSON; module snapshots
|
||||
should continue to preserve rich module values.
|
||||
|
||||
## Target Architecture
|
||||
|
||||
Each module definition should be able to declare how its output is represented
|
||||
in prompt data packages.
|
||||
|
||||
A possible shape is:
|
||||
|
||||
```go
|
||||
type ModuleDefinition struct {
|
||||
// existing fields...
|
||||
PromptExporter ModulePromptExporter
|
||||
}
|
||||
|
||||
type ModulePromptExporter func(value any) (any, error)
|
||||
```
|
||||
|
||||
The exact API may differ if implementation discovers a cleaner fit, but the
|
||||
contract should preserve these properties:
|
||||
|
||||
- the exporter is owned near the module definition or module builder;
|
||||
- the exporter receives the rich module value and returns a prompt-facing value;
|
||||
- missing exporters default to pass-through for modules whose rich value is
|
||||
already prompt-appropriate;
|
||||
- exporter errors include module ID and stanza context;
|
||||
- promptinput uses exported prompt values instead of rich values;
|
||||
- render contexts and templates continue using rich values.
|
||||
|
||||
The preferred implementation should avoid making `internal/promptinput` import
|
||||
`internal/briefing` directly. If prompt export needs registry knowledge, either:
|
||||
|
||||
- record the prompt-facing value in `module.Output` when the module snapshot is
|
||||
built; or
|
||||
- pass an explicit export map/registry into prompt-input construction without
|
||||
creating a package cycle.
|
||||
|
||||
The implementation should keep package boundaries consistent with existing
|
||||
architecture: module output policy belongs with module definitions, and data
|
||||
package serialization belongs in `internal/promptinput`.
|
||||
|
||||
## Prompt Export Contract
|
||||
|
||||
A module prompt export should be:
|
||||
|
||||
- **curated:** include fields useful to the LLM, omit fields used only for
|
||||
deterministic sentence construction;
|
||||
- **typed:** use small prompt-facing structs for modules that need reshaping;
|
||||
- **stable:** keep field names intentional and avoid duplicating equivalent
|
||||
facts under multiple names;
|
||||
- **readable:** prefer fields that explain themselves in YAML;
|
||||
- **loss-aware:** do not omit facts that the LLM needs to reason about timing,
|
||||
severity, uncertainty, or practical impact;
|
||||
- **module-owned:** keep each module responsible for its own prompt-facing
|
||||
contract.
|
||||
|
||||
Prompt-facing structs may live next to the module that owns them, for example:
|
||||
|
||||
```go
|
||||
type CurrentConditionsPromptExport struct {
|
||||
ConditionText string `json:"condition_text,omitempty"`
|
||||
TemperatureF *int `json:"temperature_f,omitempty"`
|
||||
ApparentTemperatureF *int `json:"apparent_temperature_f,omitempty"`
|
||||
RelativeHumidityPercent *int `json:"relative_humidity_percent,omitempty"`
|
||||
WindSpeedMph *int `json:"wind_speed_mph,omitempty"`
|
||||
WindDirection string `json:"wind_direction,omitempty"`
|
||||
}
|
||||
```
|
||||
|
||||
The names do not need to include `PromptExport` if implementation finds a
|
||||
clearer convention, but they should distinguish data-package shape from
|
||||
template-rendering shape.
|
||||
|
||||
## Initial Cleanup Targets
|
||||
|
||||
### Current Conditions
|
||||
|
||||
Keep prompt-facing fields that express current observed conditions directly:
|
||||
|
||||
- `condition_text`
|
||||
- `is_day`
|
||||
- temperature fields
|
||||
- apparent temperature fields
|
||||
- dewpoint fields
|
||||
- relative humidity
|
||||
- wind speed
|
||||
- one wind direction field
|
||||
|
||||
Remove prompt-facing fields that are template-only duplicates:
|
||||
|
||||
- `condition_text_lower`
|
||||
- duplicate wind-direction text when an equivalent `wind_direction` field is
|
||||
present
|
||||
|
||||
The template surface may keep those helper fields.
|
||||
|
||||
### Hourly Forecast
|
||||
|
||||
Keep prompt-facing period fields that carry facts:
|
||||
|
||||
- `period_begins`
|
||||
- `period_ends`
|
||||
- `name`
|
||||
- `is_day`
|
||||
- condition code, if useful
|
||||
- `text_description`
|
||||
- temperature fields
|
||||
- dewpoint, apparent temperature, humidity, wind, gust, pressure, visibility,
|
||||
cloud cover, precipitation probability, precipitation amount, snowfall depth,
|
||||
and UV index when provided by upstream data
|
||||
|
||||
Remove prompt-facing fields that duplicate or encode template logic:
|
||||
|
||||
- `hour_label`, because `period_begins` already gives the time in a friendly
|
||||
local label;
|
||||
- `text_description_lower`, because the LLM can interpret
|
||||
`text_description`;
|
||||
- `mention_precipitation`, because it is a template threshold helper when the
|
||||
underlying precipitation probability is present.
|
||||
|
||||
The template surface may keep these helper fields.
|
||||
|
||||
### Derived Daypart Summaries
|
||||
|
||||
Keep prompt-facing fields that describe the daypart:
|
||||
|
||||
- `date`
|
||||
- `display_name`
|
||||
- `period_begins`
|
||||
- `period_ends`
|
||||
- temperature range or the best single temperature phrase
|
||||
- apparent temperature range when useful
|
||||
- maximum precipitation probability and time
|
||||
- maximum wind gust and time
|
||||
- dominant condition
|
||||
- temperature trend
|
||||
- notable conditions
|
||||
- weather indicator booleans
|
||||
- relevant alert count
|
||||
|
||||
Remove prompt-facing fields that mainly support deterministic sentence
|
||||
construction:
|
||||
|
||||
- duplicate lower-case/display variants of the same dominant condition;
|
||||
- multiple temperature phrase fragments when a smaller set can express the same
|
||||
trend;
|
||||
- duplicate time labels where one friendly time field is enough.
|
||||
|
||||
The exact retained daypart temperature fields should be chosen during
|
||||
implementation with template needs and LLM readability in mind. The prompt
|
||||
export should preserve the facts needed to understand whether temperatures are
|
||||
rising, falling, peaking, or steady, but it does not need every phrase fragment
|
||||
used by the Markdown template.
|
||||
|
||||
### Other Modules
|
||||
|
||||
Most existing modules may initially use pass-through export unless they expose
|
||||
clear template-only helpers. During implementation, review at least:
|
||||
|
||||
- `narrative_forecast`
|
||||
- `precip_timing`
|
||||
- `outdoor_windows`
|
||||
- `alert_digest`
|
||||
- `spc_convective_outlooks`
|
||||
- `spc_convective_discussion`
|
||||
- `area_forecast_discussion`
|
||||
- `weather_story`
|
||||
- planning modules
|
||||
|
||||
Do not remove fields merely because they are verbose. Remove or reshape fields
|
||||
when they are redundant, template-specific, or confusing in the context of LLM
|
||||
input.
|
||||
|
||||
## Data Package Behavior
|
||||
|
||||
After implementation:
|
||||
|
||||
- saved YAML data packages should use prompt-facing module exports;
|
||||
- saved module snapshots should continue preserving rich module output values;
|
||||
- generated-text render contexts should continue preserving rich module values;
|
||||
- Recent Changes should continue using structured module snapshots unless a
|
||||
specific comparison should intentionally move to prompt-facing fields;
|
||||
- inspection commands should make clear whether they are showing rich module
|
||||
snapshots or prompt data packages.
|
||||
- generated-template reports, including `today`, `tomorrow`, and `daily`, should
|
||||
continue rendering from rich module values.
|
||||
|
||||
This roadmap does not require changing source warnings, report metadata,
|
||||
collected facts, derived facts, or generated report artifacts.
|
||||
|
||||
## Schema And Versioning
|
||||
|
||||
This is a prompt-input schema cleanup. Because the project is pre-release, the
|
||||
implementation may make a clean break in data-package field names without
|
||||
compatibility aliases.
|
||||
|
||||
The data-package schema version should be bumped when this feature is
|
||||
implemented because persisted data-package fields will be removed or renamed.
|
||||
This makes artifact shape changes explicit and helps inspection tooling
|
||||
distinguish old and new data packages.
|
||||
|
||||
## Documentation Guidance
|
||||
|
||||
After implementation, update implemented documentation only:
|
||||
|
||||
- `docs/internal/module.md`: describe the distinction between rich module output
|
||||
and prompt-facing export values.
|
||||
- `docs/internal/prompt-input.md`: document that data packages use curated
|
||||
prompt exports, not full template module structs.
|
||||
- `docs/templates.md`: clarify that templates may have richer fields than the
|
||||
data package.
|
||||
- Any module field examples in implemented docs should match the new
|
||||
prompt-facing data package shape.
|
||||
|
||||
Do not document future module fields or unimplemented exporters outside
|
||||
`docs/roadmap/`.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
The feature is complete when:
|
||||
|
||||
- prompt data packages serialize curated module exports instead of blindly
|
||||
serializing rich module values;
|
||||
- templates still render from rich module values without losing helper fields;
|
||||
- `current_conditions` no longer exposes lower-case condition text or duplicate
|
||||
wind-direction fields in data packages;
|
||||
- `hourly_forecast.periods[]` no longer exposes `hour_label`,
|
||||
`text_description_lower`, or `mention_precipitation` in data packages;
|
||||
- `derived_daypart_summaries` no longer exposes redundant condition and
|
||||
temperature phrase variants in data packages;
|
||||
- `today`, `tomorrow`, and `daily` rendered reports continue to have access to
|
||||
rich daypart helper fields for deterministic template wording;
|
||||
- simple modules that do not need cleanup still export correctly through default
|
||||
pass-through behavior;
|
||||
- exporter errors include module/stanza context;
|
||||
- YAML category ordering remains unchanged;
|
||||
- module snapshot artifacts remain rich enough for templates, inspection, and
|
||||
regression diagnosis;
|
||||
- tests prove that removed prompt-facing fields are absent from saved YAML data
|
||||
packages and still available to templates where needed.
|
||||
|
||||
## Testing Expectations
|
||||
|
||||
Implementation should add or update focused tests for:
|
||||
|
||||
- module registry validation for prompt exporters, if exporters are registered
|
||||
there;
|
||||
- promptinput construction using exported prompt values;
|
||||
- pass-through behavior for simple modules;
|
||||
- custom exports for current conditions, hourly forecast, and daypart summaries;
|
||||
- data-package YAML output rejecting stale fields;
|
||||
- template render tests proving template-only helper fields remain available for
|
||||
`today`, `tomorrow`, and `daily`;
|
||||
- app workflow tests proving saved data packages use curated exports while
|
||||
render contexts keep rich values.
|
||||
|
||||
Suggested validation after implementation:
|
||||
|
||||
```sh
|
||||
go test ./internal/module ./internal/briefing ./internal/promptinput
|
||||
go test ./internal/generatedtext ./internal/reporttemplate ./internal/app
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Deferred Work
|
||||
|
||||
Do not include these in the initial cleanup unless implementation reveals they
|
||||
are necessary:
|
||||
|
||||
- user-configurable data-package field selection;
|
||||
- per-prompt custom field profiles;
|
||||
- reflection-based generic include/exclude lists;
|
||||
- automatic schema generation for data-package exports;
|
||||
- changing collected facts or derived facts contracts;
|
||||
- changing Scriptorium invocation behavior;
|
||||
- changing generated Markdown templates beyond preserving their current output.
|
||||
|
||||
## Open Questions
|
||||
|
||||
No open questions block implementation.
|
||||
|
||||
The implementation plan in `docs/roadmap/implementation.md` is the sequencing
|
||||
authority for this feature.
|
||||
@@ -1,657 +0,0 @@
|
||||
# Data Package Export Implementation Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
This roadmap defines the staged implementation plan for
|
||||
`docs/roadmap/data-package-exports.md`. It is written for an LLM coding agent
|
||||
that will implement the curated data-package export feature in order.
|
||||
|
||||
The goal is to separate rich module values used by templates from curated
|
||||
prompt-facing values serialized into Scriptorium data packages. The change
|
||||
should make YAML data packages smaller and clearer without reducing deterministic
|
||||
template composability.
|
||||
|
||||
This is a planning document only. It may describe unimplemented behavior because
|
||||
it lives under `docs/roadmap/`.
|
||||
|
||||
## Source Roadmap
|
||||
|
||||
Use `docs/roadmap/data-package-exports.md` as the feature roadmap and target
|
||||
state authority. That document defines the user intent, module/export boundary,
|
||||
initial cleanup targets, and non-goals.
|
||||
|
||||
If this implementation plan and the feature roadmap conflict, update the feature
|
||||
roadmap first so it remains the conceptual source of truth, then update this
|
||||
file.
|
||||
|
||||
## Locked Decisions
|
||||
|
||||
- Keep rich module values available for templates, module snapshots, inspection,
|
||||
Recent Changes, and render contexts.
|
||||
- Serialize curated prompt-facing module values into data packages.
|
||||
- Store prompt-facing values on `module.Output` as runtime-only values.
|
||||
- Do not persist prompt-facing values in module snapshot JSON.
|
||||
- Keep `internal/promptinput` independent from `internal/briefing`.
|
||||
- Add prompt export behavior to the module registry/definition path where module
|
||||
output policy already lives.
|
||||
- Default modules without custom exporters to pass-through prompt values.
|
||||
- Bump the data-package schema version from `weatherreporter.data_package.v2` to
|
||||
`weatherreporter.data_package.v3`.
|
||||
- No compatibility aliases are required for removed prompt-facing fields.
|
||||
- Do not add user-configurable field selection, field allowlists, reflection
|
||||
filters, or prompt-specific field profiles in this implementation.
|
||||
|
||||
## Implementation Principles
|
||||
|
||||
- Preserve existing generated Markdown output.
|
||||
- Preserve existing template render-context richness.
|
||||
- Preserve existing module snapshot richness and JSON shape except for any
|
||||
unavoidable schema-version-only change. The preferred approach is to omit
|
||||
prompt values from module snapshot JSON entirely.
|
||||
- Keep exporter code near the module that owns the shape.
|
||||
- Use typed prompt-facing structs for modules that need reshaping.
|
||||
- Keep simple modules on default pass-through behavior.
|
||||
- Make exporter errors actionable and include module ID/stanza context.
|
||||
- Keep YAML category ordering unchanged.
|
||||
- Treat generated-template `today`, `tomorrow`, and `daily` reports as equal
|
||||
consumers of rich template module values.
|
||||
- Update implemented docs only after code behavior exists.
|
||||
|
||||
## Stage 1: Runtime Prompt Value Contract
|
||||
|
||||
### Goal
|
||||
|
||||
Add a runtime-only prompt value to module outputs while preserving rich module
|
||||
snapshot persistence.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/module/module.go`
|
||||
- `internal/module/module_test.go`
|
||||
- `internal/state/filesystem.go`
|
||||
- `internal/state/filesystem_test.go`
|
||||
- `internal/app/app.go`
|
||||
- `internal/app/app_test.go`
|
||||
|
||||
### Implementation
|
||||
|
||||
- Add a runtime-only field to `module.Output`, for example:
|
||||
|
||||
```go
|
||||
type Output struct {
|
||||
ID ID `json:"id"`
|
||||
StanzaName string `json:"stanzaName"`
|
||||
Value any `json:"value"`
|
||||
PromptValue any `json:"-"`
|
||||
}
|
||||
```
|
||||
|
||||
- Add a small helper in `internal/module`, for example:
|
||||
|
||||
```go
|
||||
func (o Output) DataPackageValue() any
|
||||
```
|
||||
|
||||
The helper should return `PromptValue` when non-nil and fall back to `Value`
|
||||
otherwise. This fallback keeps tests and any manually built snapshots simple.
|
||||
|
||||
- Do not require `PromptValue` in `Snapshot.Validate`.
|
||||
- Do not persist `PromptValue` to module snapshot JSON.
|
||||
- Do not change `StanzaValue`; it should continue decoding rich `Value`.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Module snapshots still serialize rich module values under `value`.
|
||||
- Module snapshots do not serialize `promptValue` or any equivalent field.
|
||||
- Existing rich-module snapshot lookup and `StanzaValue` behavior remain
|
||||
unchanged.
|
||||
- Code has a single helper for choosing the data-package value from an output.
|
||||
|
||||
### Tests
|
||||
|
||||
- Add module tests proving:
|
||||
- `DataPackageValue` uses `PromptValue` when set;
|
||||
- `DataPackageValue` falls back to `Value`;
|
||||
- marshaled snapshot JSON omits `PromptValue`;
|
||||
- `StanzaValue` continues decoding rich `Value`.
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/module ./internal/state
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 2: Module Registry Prompt Exporters
|
||||
|
||||
### Goal
|
||||
|
||||
Teach the module registry to attach prompt-facing values to outputs as modules
|
||||
are built.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/briefing/modules.go`
|
||||
- `internal/briefing/modules_test.go`
|
||||
- `internal/briefing/*_module.go`
|
||||
- `internal/module/module.go`
|
||||
|
||||
### Implementation
|
||||
|
||||
- Add a prompt exporter type in `internal/briefing`, for example:
|
||||
|
||||
```go
|
||||
type ModulePromptExporter func(value any) (any, error)
|
||||
```
|
||||
|
||||
- Add `PromptExporter ModulePromptExporter` to `ModuleDefinition`.
|
||||
- In `ModuleRegistry.BuildModule`, after builder output ID and stanza validation:
|
||||
- if `PromptExporter` is nil, set `output.PromptValue = output.Value`;
|
||||
- if `PromptExporter` is present, call it with `output.Value`;
|
||||
- set `output.PromptValue` to the returned value;
|
||||
- wrap exporter errors with module ID and stanza context.
|
||||
- Add a typed exporter helper if it keeps module exporters concise, for example:
|
||||
|
||||
```go
|
||||
func promptExporter[T any](fn func(T) (any, error)) ModulePromptExporter
|
||||
```
|
||||
|
||||
The helper should convert `any` through JSON marshal/unmarshal or direct type
|
||||
assertion only if it meaningfully reduces boilerplate without hiding errors.
|
||||
|
||||
- Do not make `internal/promptinput` import `internal/briefing`.
|
||||
- Do not move module-specific export policy into `internal/promptinput`.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Every built module output has a non-nil data-package value.
|
||||
- Modules without custom exporters pass through rich values.
|
||||
- Custom exporter errors identify the module and stanza.
|
||||
- Registry validation remains focused on definitions, duplicate IDs/stanzas,
|
||||
supported reports, options, builders, and missing-data policy.
|
||||
|
||||
### Tests
|
||||
|
||||
- Add or update module registry tests for:
|
||||
- default pass-through prompt values;
|
||||
- custom exporter prompt values;
|
||||
- exporter error wrapping;
|
||||
- output ID/stanza validation still runs before or around export behavior;
|
||||
- output `PromptValue` is not persisted in snapshot JSON.
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/briefing ./internal/module
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 3: Promptinput Uses Exported Values And Schema V3
|
||||
|
||||
### Goal
|
||||
|
||||
Make data-package construction serialize prompt-facing values and bump the data
|
||||
package schema version.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/promptinput/package.go`
|
||||
- `internal/promptinput/package_test.go`
|
||||
- `internal/state/filesystem_test.go`
|
||||
- `internal/app/app.go`
|
||||
- `internal/app/app_test.go`
|
||||
- `docs/roadmap/data-package-exports.md`
|
||||
|
||||
### Implementation
|
||||
|
||||
- Change `promptinput.SchemaVersion` to:
|
||||
|
||||
```go
|
||||
const SchemaVersion = "weatherreporter.data_package.v3"
|
||||
```
|
||||
|
||||
- Update `stanzasFromSnapshot` to use `output.DataPackageValue()` rather than
|
||||
`output.Value`.
|
||||
- Keep `BriefingStanzas` category grouping and ordering unchanged.
|
||||
- Keep `LoadYAML` validation strict for the current schema version.
|
||||
- Update tests and fixtures that assert `weatherreporter.data_package.v2`.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Saved data packages use schema version `weatherreporter.data_package.v3`.
|
||||
- Data package stanzas use `PromptValue` when present.
|
||||
- Data package stanzas fall back to rich `Value` when `PromptValue` is absent,
|
||||
which keeps hand-built test snapshots and loaded rich snapshots usable.
|
||||
- YAML category ordering remains unchanged.
|
||||
- Module snapshots remain rich and unaffected by prompt export serialization.
|
||||
|
||||
### Tests
|
||||
|
||||
- Update promptinput tests for:
|
||||
- schema version `v3`;
|
||||
- prompt value preferred over rich value;
|
||||
- fallback to rich value;
|
||||
- deterministic categorized YAML output unchanged apart from stanza values and
|
||||
schema version;
|
||||
- unknown/uncategorized stanza behavior unchanged.
|
||||
- Update state tests that load data packages.
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/promptinput ./internal/state
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small enough for one implementation prompt.
|
||||
|
||||
## Stage 4: Current Conditions And Hourly Forecast Exports
|
||||
|
||||
### Goal
|
||||
|
||||
Add custom prompt exports for the clearest noisy raw-data modules:
|
||||
`current_conditions` and `hourly_forecast`.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/briefing/current_conditions_module.go`
|
||||
- `internal/briefing/hourly_forecast_module.go`
|
||||
- `internal/briefing/base_modules_test.go`
|
||||
- `internal/generatedtext/render_context.go`
|
||||
- `internal/reporttemplate/templates/*.md.tmpl`
|
||||
- `internal/app/app_test.go`
|
||||
|
||||
### Current Conditions Export Shape
|
||||
|
||||
The prompt-facing `current_conditions` export should keep:
|
||||
|
||||
- `condition_text`
|
||||
- `is_day`
|
||||
- `temperature_c`
|
||||
- `temperature_f`
|
||||
- `apparent_temperature_c`
|
||||
- `apparent_temperature_f`
|
||||
- `dewpoint_c`
|
||||
- `dewpoint_f`
|
||||
- `relative_humidity_percent`
|
||||
- `wind_speed_kmh`
|
||||
- `wind_speed_mph`
|
||||
- `wind_direction`
|
||||
|
||||
It should remove:
|
||||
|
||||
- `condition_text_lower`
|
||||
- `wind_direction_text`
|
||||
|
||||
The rich `CurrentConditionsModule` should keep those helper fields for
|
||||
templates.
|
||||
|
||||
### Hourly Forecast Export Shape
|
||||
|
||||
The prompt-facing `hourly_forecast` export should keep module-level fields:
|
||||
|
||||
- `product`
|
||||
- `issued_at`
|
||||
- `updated_at`
|
||||
- `source_location`
|
||||
- `source_location_id`
|
||||
- `periods`
|
||||
|
||||
Each prompt-facing hourly period should keep:
|
||||
|
||||
- `period_begins`
|
||||
- `period_ends`
|
||||
- `name`
|
||||
- `is_day`
|
||||
- `condition_code`
|
||||
- `text_description`
|
||||
- temperature fields
|
||||
- dewpoint fields
|
||||
- wind speed and gust fields
|
||||
- `wind_direction`
|
||||
- pressure fields
|
||||
- visibility fields
|
||||
- apparent temperature fields
|
||||
- `cloud_cover_percent`
|
||||
- `probability_of_precipitation_percent`
|
||||
- precipitation amount fields
|
||||
- snowfall depth fields
|
||||
- `uv_index`
|
||||
- `relative_humidity_percent`
|
||||
|
||||
Each prompt-facing hourly period should remove:
|
||||
|
||||
- `hour_label`
|
||||
- `text_description_lower`
|
||||
- `mention_precipitation`
|
||||
|
||||
The rich `HourlyForecastPeriod` should keep those helper fields for templates.
|
||||
|
||||
### Implementation
|
||||
|
||||
- Add prompt export structs near each module.
|
||||
- Add exporter functions near each module.
|
||||
- Register exporters in the default module definitions.
|
||||
- Prefer straightforward field copying over reflection.
|
||||
- Preserve `omitempty` behavior.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Saved data packages no longer include removed fields for current conditions or
|
||||
hourly forecast.
|
||||
- Rich module snapshots and render contexts still include template helper
|
||||
fields.
|
||||
- Existing hourly, today, tomorrow, and daily template output remains unchanged
|
||||
wherever those templates consume current conditions or hourly forecast values.
|
||||
|
||||
### Tests
|
||||
|
||||
- Update module tests to prove prompt exports omit stale fields and keep
|
||||
expected factual fields.
|
||||
- Update template/render-context tests to prove helper fields remain available
|
||||
to templates.
|
||||
- Update app workflow tests to inspect saved YAML and reject:
|
||||
- `condition_text_lower`;
|
||||
- `wind_direction_text`;
|
||||
- `hour_label`;
|
||||
- `text_description_lower`;
|
||||
- `mention_precipitation`.
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/briefing ./internal/generatedtext ./internal/reporttemplate ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Medium. Suitable for one implementation prompt.
|
||||
|
||||
## Stage 5: Derived Daypart Summary Export
|
||||
|
||||
### Goal
|
||||
|
||||
Add a curated prompt export for `derived_daypart_summaries` while preserving the
|
||||
rich daypart fields used by Daily, Today, and Tomorrow templates.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/briefing/derived_daypart_summaries_module.go`
|
||||
- `internal/briefing/derived_modules_test.go`
|
||||
- `internal/generatedtext/render_context.go`
|
||||
- `internal/reporttemplate/templates/daily.md.tmpl`
|
||||
- `internal/reporttemplate/templates/today.md.tmpl`
|
||||
- `internal/reporttemplate/templates/tomorrow.md.tmpl`
|
||||
- `internal/reporttemplate/reporttemplate_test.go`
|
||||
- `internal/app/app_test.go`
|
||||
|
||||
### Export Shape
|
||||
|
||||
For each daypart, the prompt-facing export should keep:
|
||||
|
||||
- `date`
|
||||
- `display_name`
|
||||
- `period_begins`
|
||||
- `period_ends`
|
||||
- `temp_range_f`
|
||||
- `apparent_temp_range_f`
|
||||
- `max_pop_percent`
|
||||
- `max_pop_time`
|
||||
- `mention_precipitation`
|
||||
- `max_wind_gust_mph`
|
||||
- `max_wind_gust_time`
|
||||
- `dominant_condition`
|
||||
- `temperature_trend`
|
||||
- `temperature_start_phrase_f`
|
||||
- `temperature_end_phrase_f`
|
||||
- `temperature_peak_phrase_f`
|
||||
- `temperature_steady_phrase_f`
|
||||
- `notable_conditions`
|
||||
- `snow`
|
||||
- `ice`
|
||||
- `fog`
|
||||
- `heat`
|
||||
- `cold`
|
||||
- `wind`
|
||||
- `relevant_alert_count`
|
||||
|
||||
The prompt-facing export should remove:
|
||||
|
||||
- `temperature_phrase_f`
|
||||
- `dominant_condition_lower`
|
||||
- `dominant_condition_display`
|
||||
- `max_pop_time_label`
|
||||
|
||||
For `max_pop_time`, use the most readable existing time label. Prefer
|
||||
`MaxPopTimeLabel` when present, falling back to `MaxPopTime`, while keeping the
|
||||
prompt-facing field name `max_pop_time`.
|
||||
|
||||
Keep the temperature trend phrase fields even though only some are populated for
|
||||
each trend. They are not duplicates when used according to the trend:
|
||||
|
||||
- rising/falling use start and end phrases;
|
||||
- peaking uses peak phrase;
|
||||
- steady uses steady phrase.
|
||||
|
||||
### Implementation
|
||||
|
||||
- Add prompt export structs near the daypart module.
|
||||
- Add an exporter for the `map[string]DerivedDaypartSummaryModule` value.
|
||||
- Preserve map keys and values for all emitted dayparts.
|
||||
- Register the exporter in the default module definitions.
|
||||
- Do not alter rich `DerivedDaypartSummaryModule` fields used by templates.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Saved data packages no longer include the removed daypart fields.
|
||||
- The prompt-facing daypart export still gives the LLM enough information to
|
||||
understand condition, temperature trend, precipitation, wind, notable
|
||||
conditions, and alert relevance.
|
||||
- Daily, Today, and Tomorrow template output remains unchanged.
|
||||
- Render contexts still expose rich daypart helper fields.
|
||||
|
||||
### Tests
|
||||
|
||||
- Update daypart module tests for:
|
||||
- prompt export field presence;
|
||||
- removed field absence;
|
||||
- rising, falling, peaking, and steady trend values;
|
||||
- max PoP time using the friendly label under the stable `max_pop_time` key.
|
||||
- Update template tests to prove rich helper fields still render Daily, Today,
|
||||
and Tomorrow daypart wording.
|
||||
- Update app data-package tests to reject stale daypart fields.
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/briefing ./internal/generatedtext ./internal/reporttemplate ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Medium. Suitable for one implementation prompt.
|
||||
|
||||
## Stage 6: Pass-Through Review And Workflow Regression Tests
|
||||
|
||||
### Goal
|
||||
|
||||
Confirm all other modules export correctly through pass-through behavior and add
|
||||
workflow-level coverage proving the new boundary.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `internal/briefing/*_module.go`
|
||||
- `internal/briefing/modules.go`
|
||||
- `internal/promptinput/package_test.go`
|
||||
- `internal/app/app_test.go`
|
||||
- `internal/generatedtext/render_context_test.go`
|
||||
- `internal/reporttemplate/reporttemplate_test.go`
|
||||
|
||||
### Implementation
|
||||
|
||||
- Review remaining modules:
|
||||
- `metadata`
|
||||
- `narrative_forecast`
|
||||
- `derived_daily_summary`
|
||||
- `precip_timing`
|
||||
- `outdoor_windows`
|
||||
- `alert_digest`
|
||||
- `spc_convective_outlooks`
|
||||
- `spc_convective_discussion`
|
||||
- `area_forecast_discussion`
|
||||
- `weather_story`
|
||||
- planning modules
|
||||
- Keep pass-through behavior for modules that are already prompt-appropriate.
|
||||
- Add custom exporters only if a module contains clear template-only helpers or
|
||||
confusing duplicate fields.
|
||||
- Do not broaden this stage into a general prompt-schema redesign.
|
||||
- Add workflow assertions that:
|
||||
- module snapshots contain rich values;
|
||||
- render contexts contain rich values;
|
||||
- data packages contain curated values;
|
||||
- Scriptorium receives the curated data package path exactly as before.
|
||||
- Include workflow coverage for generated-template `today`, `tomorrow`, and
|
||||
`daily` reports when asserting daypart and render-context behavior.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Every default module either has a custom exporter or intentionally uses
|
||||
pass-through.
|
||||
- App workflow tests prove saved data packages omit the cleaned fields.
|
||||
- App workflow tests prove rich helper fields remain available where templates
|
||||
use them.
|
||||
- App workflow or render-context tests include `daily` alongside `today` and
|
||||
`tomorrow` for daypart helper coverage.
|
||||
- Existing report generation behavior remains unchanged except for data-package
|
||||
YAML content and schema version.
|
||||
|
||||
### Tests
|
||||
|
||||
Suggested focused command:
|
||||
|
||||
```sh
|
||||
go test ./internal/briefing ./internal/promptinput ./internal/generatedtext ./internal/reporttemplate ./internal/app
|
||||
```
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Small to medium. Suitable for one implementation prompt.
|
||||
|
||||
## Stage 7: Documentation And Final Validation
|
||||
|
||||
### Goal
|
||||
|
||||
Update implemented documentation after the code exists and run full validation.
|
||||
|
||||
### Files To Inspect
|
||||
|
||||
- `docs/internal/module.md`
|
||||
- `docs/internal/prompt-input.md`
|
||||
- `docs/templates.md`
|
||||
- `docs/operations.md`
|
||||
- `docs/troubleshooting.md`
|
||||
- `docs/roadmap/data-package-exports.md`
|
||||
- tests that assert documentation examples or data-package snippets
|
||||
|
||||
### Implementation
|
||||
|
||||
- Update `docs/internal/module.md` to describe:
|
||||
- rich module output;
|
||||
- runtime prompt export values;
|
||||
- pass-through exporters;
|
||||
- module-owned prompt export policy.
|
||||
- Update `docs/internal/prompt-input.md` to document:
|
||||
- schema version `weatherreporter.data_package.v3`;
|
||||
- curated module export behavior;
|
||||
- category ordering unchanged;
|
||||
- data packages are not full template render contexts.
|
||||
- Update `docs/templates.md` to clarify that templates can use richer fields
|
||||
than the data package exposes.
|
||||
- Update any Daily template-variable documentation alongside Today and Tomorrow
|
||||
references where the same module fields are discussed.
|
||||
- Update any implemented docs containing stale examples of removed data-package
|
||||
fields.
|
||||
- Do not document deferred configurable field profiles or reflection filters as
|
||||
implemented behavior.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Non-roadmap docs describe only implemented data-package export behavior.
|
||||
- Docs do not imply data packages contain template-only helper fields.
|
||||
- Examples and snippets use schema version `v3` when they show data packages.
|
||||
|
||||
### Validation Commands
|
||||
|
||||
```sh
|
||||
go test ./internal/module ./internal/briefing ./internal/promptinput
|
||||
go test ./internal/generatedtext ./internal/reporttemplate ./internal/app
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
### Stale Field Greps
|
||||
|
||||
Run focused checks against docs and expected saved data-package fixtures/snippets:
|
||||
|
||||
```sh
|
||||
rg -n "condition_text_lower|wind_direction_text|hour_label|text_description_lower|mention_precipitation|dominant_condition_lower|dominant_condition_display|max_pop_time_label|temperature_phrase_f" docs internal/*/*_test.go
|
||||
```
|
||||
|
||||
Review matches manually. Some matches should remain in rich module structs,
|
||||
template tests, and render-context tests; they should not remain as expected
|
||||
data-package fields.
|
||||
|
||||
### Prompt Size
|
||||
|
||||
Medium. Suitable for one implementation prompt.
|
||||
|
||||
## Deferred Work
|
||||
|
||||
Do not include these in this implementation sequence:
|
||||
|
||||
- user-configurable data-package field selection;
|
||||
- per-prompt custom field profiles;
|
||||
- reflection-based generic include/exclude lists;
|
||||
- automatic schema generation for data-package exports;
|
||||
- changing collected facts or derived facts contracts;
|
||||
- changing Scriptorium invocation behavior;
|
||||
- changing generated Markdown templates beyond preserving their output;
|
||||
- migrating old workspace data-package artifacts.
|
||||
|
||||
## Open Questions
|
||||
|
||||
No open questions block implementation.
|
||||
|
||||
The implementation plan intentionally locks in the recommended choices from the
|
||||
feature roadmap: schema version bump to `weatherreporter.data_package.v3`, and
|
||||
runtime-only prompt export values stored on `module.Output`.
|
||||
|
||||
## Global Validation Checklist
|
||||
|
||||
- `go test ./...` passes.
|
||||
- `go run ./cmd/weatherreporter --help` is accurate.
|
||||
- `git diff --check` passes.
|
||||
- Saved data packages use schema version `weatherreporter.data_package.v3`.
|
||||
- Saved data packages serialize prompt-facing module exports.
|
||||
- Saved module snapshots serialize rich module values and do not persist
|
||||
prompt-facing values.
|
||||
- Render contexts continue to expose rich module values.
|
||||
- Hourly, Today, Tomorrow, and Daily templates continue to render the same
|
||||
Markdown output.
|
||||
- YAML briefing category order remains unchanged.
|
||||
- `current_conditions` data packages omit `condition_text_lower` and
|
||||
`wind_direction_text`.
|
||||
- `hourly_forecast.periods[]` data packages omit `hour_label`,
|
||||
`text_description_lower`, and `mention_precipitation`.
|
||||
- `derived_daypart_summaries` data packages omit `temperature_phrase_f`,
|
||||
`dominant_condition_lower`, `dominant_condition_display`, and
|
||||
`max_pop_time_label`.
|
||||
- Non-roadmap docs describe only implemented behavior.
|
||||
Reference in New Issue
Block a user