Compare commits
2 Commits
bcb4a64c68
...
6915bf1ba2
| Author | SHA1 | Date | |
|---|---|---|---|
| 6915bf1ba2 | |||
| ac6ede8f9c |
553
docs/roadmap/audit.md
Normal file
553
docs/roadmap/audit.md
Normal file
@@ -0,0 +1,553 @@
|
||||
# Code Quality And Deduplication Audit
|
||||
|
||||
## 1. Executive summary
|
||||
|
||||
Overall code quality is good. The implementation is small, direct, and aligned
|
||||
with the documented architecture: CLI parsing is isolated in `internal/cli`,
|
||||
configuration is centralized in `internal/config`, report definitions live in
|
||||
`internal/report`, external systems sit behind adapters, and most domain logic
|
||||
is covered by focused tests.
|
||||
|
||||
The codebase appears ready for a limited cleanup pass before the next major
|
||||
release. I did not find a major architectural risk that requires a broad
|
||||
rewrite.
|
||||
|
||||
Top three refactoring targets:
|
||||
|
||||
1. Centralize atomic file and JSON artifact writes. Similar write patterns now
|
||||
exist in state, briefing, prompt input, Weather API bundle saving,
|
||||
Scriptorium render saving, and report-copy code.
|
||||
2. Move report artifact naming, grouping, and compatibility policy closer to
|
||||
the report registry. Related decisions are split across `internal/report`,
|
||||
`internal/state`, and `internal/app`.
|
||||
3. Reduce repeated CLI/app inspection and config-loading scaffolding. The
|
||||
inspect commands repeat parse/load/call/write patterns, and app inspection
|
||||
repeats store and metadata lookup flows.
|
||||
|
||||
Recommended cleanup should be incremental and behavior-preserving. Avoid a
|
||||
generic workflow engine, plugin layer, CLI redesign, or broad storage rewrite.
|
||||
|
||||
## 2. Repository map reviewed
|
||||
|
||||
Reviewed documentation and policy:
|
||||
|
||||
- `README.md`
|
||||
- `docs/policy/architecture.md`
|
||||
- `docs/policy/development.md`
|
||||
- `docs/config.md`
|
||||
- `docs/cli.md`
|
||||
- `docs/operations.md`
|
||||
- `docs/troubleshooting.md`
|
||||
- `docs/internal/*`
|
||||
- `docs/integrations/*`
|
||||
- `docs/roadmap/*`
|
||||
- `examples/config.yml`
|
||||
- `examples/minimal-config.yml`
|
||||
|
||||
Reviewed implementation packages:
|
||||
|
||||
- `cmd/weatherreporter`
|
||||
- `internal/cli`
|
||||
- `internal/config`
|
||||
- `internal/app`
|
||||
- `internal/report`
|
||||
- `internal/forecast`
|
||||
- `internal/briefing`
|
||||
- `internal/promptinput`
|
||||
- `internal/changes`
|
||||
- `internal/state`
|
||||
- `internal/timeutil`
|
||||
- `internal/adapters/weatherapi`
|
||||
- `internal/adapters/scriptorium`
|
||||
|
||||
Reviewed major execution paths:
|
||||
|
||||
- `weatherreporter generate daily`
|
||||
- `weatherreporter generate tomorrow`
|
||||
- `weatherreporter generate three-day`
|
||||
- `weatherreporter generate weekend`
|
||||
- `weatherreporter generate storm`
|
||||
- `weatherreporter run morning`
|
||||
- `weatherreporter run evening`
|
||||
- `weatherreporter inspect reports`
|
||||
- `weatherreporter inspect metadata`
|
||||
- `weatherreporter inspect briefing`
|
||||
- `weatherreporter inspect data-package`
|
||||
- `weatherreporter inspect prior`
|
||||
- `weatherreporter inspect sources`
|
||||
|
||||
Reviewed tests:
|
||||
|
||||
- CLI parser and workflow tests under `internal/cli`
|
||||
- config/default/example tests under `internal/config`
|
||||
- app workflow tests under `internal/app`
|
||||
- adapter tests under `internal/adapters/*`
|
||||
- state path and prior-snapshot tests under `internal/state`
|
||||
- report period tests under `internal/report`
|
||||
- package-level tests for forecast, briefing, prompt input, and changes
|
||||
|
||||
Areas not present in this repository: `internal/stage`, `internal/modules`,
|
||||
`internal/validators`, `internal/storage`, `internal/artifacts`,
|
||||
`internal/manifest`, `internal/schema`, `pkg`, remote storage, object-store
|
||||
keys, and persistent progress manifests.
|
||||
|
||||
## 3. High-confidence deduplication opportunities
|
||||
|
||||
### Centralize atomic file and JSON artifact writes
|
||||
|
||||
- Affected files/packages: `internal/state/filesystem.go`,
|
||||
`internal/briefing/package.go`, `internal/promptinput/package.go`,
|
||||
`internal/adapters/weatherapi/client.go`,
|
||||
`internal/adapters/scriptorium/runner.go`, `internal/app/app.go`.
|
||||
- Duplicated or near-duplicated behavior: multiple functions marshal JSON,
|
||||
create parent directories, create temp files in the target directory, write,
|
||||
close, rename, and defer temp-file cleanup. `copyFileAtomic` repeats the
|
||||
same write path for Markdown report copies. `scriptorium.SaveRenderResult`
|
||||
duplicates state preflight saving and does not appear to be used by the app.
|
||||
- Why it matters: atomic write behavior is part of the state and recovery
|
||||
contract. A future bug fix around permissions, fsync behavior, temp-file
|
||||
cleanup, Windows rename behavior, or error context would need to be applied
|
||||
in several places.
|
||||
- Recommended refactor: add a small internal file helper, likely under
|
||||
`internal/state` if kept state-specific or a narrow `internal/fileutil`
|
||||
package if used by adapters too. Provide helpers such as `WriteFileAtomic`,
|
||||
`WriteJSONAtomic`, and possibly `CopyFileAtomic`. Then remove unused
|
||||
duplicate save helpers or route them through the shared helper.
|
||||
- Suggested tests: keep existing state, app, adapter, briefing, and prompt input
|
||||
save tests. Add one focused helper test for parent-directory creation,
|
||||
overwrite behavior, and temp-file cleanup on write errors if the helper is in
|
||||
a new package.
|
||||
- Risk level: low. This is a behavior-preserving mechanical cleanup if error
|
||||
messages are kept compatible where tests assert them.
|
||||
|
||||
### Centralize report artifact grouping, naming, and compatibility policy
|
||||
|
||||
- Affected files/packages: `internal/report`, `internal/state/filesystem.go`,
|
||||
`internal/app/app.go`, state and app tests.
|
||||
- Duplicated or near-duplicated behavior: `internal/report` owns report IDs,
|
||||
prompt IDs, default output names, batches, and comparison strategies.
|
||||
`internal/state` separately maps report IDs to artifact groups such as
|
||||
`daily`, `three-day`, `weekend`, and `storm`. `internal/app` separately
|
||||
converts `DefaultOutputName` underscores to hyphens for batch output copies.
|
||||
`internal/state` also hardcodes compatible prior report matching instead of
|
||||
asking the report catalog.
|
||||
- Why it matters: adding or renaming a report requires updates in several
|
||||
packages. A bug in grouping or compatibility could affect state lookup,
|
||||
Recent Changes, managed paths, and batch output names.
|
||||
- Recommended refactor: extend `report.Definition` or add registry helpers for
|
||||
artifact group, managed output group, batch copy filename, and compatible
|
||||
prior report IDs. Keep filesystem path joining in `internal/state`, but move
|
||||
report identity policy out of state/app. Preserve current path strings and
|
||||
output filenames.
|
||||
- Suggested tests: strengthen `internal/report` tests to assert each definition
|
||||
exposes group, copy filename, and compatibility policy. Keep existing
|
||||
`internal/state` path tests and batch output tests as regression coverage.
|
||||
- Risk level: medium-low. The behavior is user-visible through artifact paths
|
||||
and `--out-dir` names, so preserve exact strings.
|
||||
|
||||
### Collapse repeated inspect command and app lookup flow
|
||||
|
||||
- Affected files/packages: `internal/cli/root.go`, `internal/app/inspect.go`,
|
||||
CLI and app inspect tests.
|
||||
- Duplicated or near-duplicated behavior: each inspect subcommand repeats flag
|
||||
parsing, `config.Load`, app call, error handling, and JSON writing. The app
|
||||
inspect functions repeatedly create the default store, load metadata by
|
||||
RunID, and then load or derive a specific result.
|
||||
- Why it matters: adding another inspect view or changing config-loading/error
|
||||
behavior would require edits in multiple cases. It also increases the chance
|
||||
that one inspect command gains different output or error behavior.
|
||||
- Recommended refactor: add a small inspect command table in `internal/cli`
|
||||
mapping command name to parser and handler. In `internal/app`, add an
|
||||
internal helper that returns the store, metadata, and metadata path for a
|
||||
RunID, then build briefing/data-package/prior/source views from that helper.
|
||||
- Suggested tests: keep the existing `TestRunInspectGeneratedArtifacts` and
|
||||
missing metadata tests. Add a focused table test that each registered inspect
|
||||
command accepts `--config` and rejects missing RunID where applicable.
|
||||
- Risk level: low. The refactor is local and should preserve public CLI output.
|
||||
|
||||
### Remove or resolve unused report output config surface
|
||||
|
||||
- Affected files/packages: `internal/config/config.go`,
|
||||
`internal/config/load.go`, `internal/config/defaults.go`,
|
||||
`internal/config/validate.go`, docs and examples if behavior changes.
|
||||
- Duplicated or near-duplicated behavior: `ReportOutputConfig` contains
|
||||
`OutputDir` and `Paths`, and `LoadOptions.Output` writes to
|
||||
`cfg.Reports.OutputDir`. Current command behavior uses `GenerateRequest`
|
||||
`OutputPath` and `BatchRequest.OutputDir`; docs correctly say `--out` and
|
||||
`--out-dir` control extra copies without changing config files. The report
|
||||
output config fields do not appear to drive implemented behavior.
|
||||
- Why it matters: unused config fields are a maintenance hazard. They invite
|
||||
future docs drift and make it unclear whether output policy belongs in config,
|
||||
CLI requests, report definitions, or state.
|
||||
- Recommended refactor: decide one behavior before release. Either remove the
|
||||
unused config surface and the `LoadOptions.Output` mutation, or implement
|
||||
`reports.output_dir`/`reports.paths` as real defaults for generated extra
|
||||
copies. Given current docs and CLI behavior, removal is the lower-risk option.
|
||||
- Suggested tests: config tests should assert only implemented config fields.
|
||||
CLI tests should continue to cover `--out` and `--out-dir`.
|
||||
- Risk level: medium. This touches config structures and may affect users if
|
||||
anyone has already copied old config fields, so pair the change with clear
|
||||
release notes if removed.
|
||||
|
||||
## 4. Medium-confidence opportunities
|
||||
|
||||
### Reduce Weather API source fetch boilerplate
|
||||
|
||||
- Affected files/packages: `internal/adapters/weatherapi/client.go`,
|
||||
weather adapter tests.
|
||||
- Duplicated or near-duplicated behavior: optional source fetch methods repeat
|
||||
request construction, missing handling, JSON decode, malformed-source policy,
|
||||
source timestamp assignment, bundle assignment, and source recording. Hourly
|
||||
is intentionally different because it is required and validates periods.
|
||||
- Why it matters: adding another Weather API source will likely copy this
|
||||
structure and may accidentally diverge on missing-source policy or provenance.
|
||||
- Recommended refactor: introduce a small source specification/helper for
|
||||
optional typed sources. Keep hourly special. Do not build a generic HTTP
|
||||
ingestion framework.
|
||||
- Suggested tests: keep existing endpoint/query/missing-source tests. Add one
|
||||
test that optional malformed data follows the configured missing-source
|
||||
policy across at least two source specs.
|
||||
- Risk level: medium. The current explicit functions are readable; only refactor
|
||||
if adding more source types or touching missing-source behavior.
|
||||
|
||||
### Share Scriptorium render/run execution scaffolding
|
||||
|
||||
- Affected files/packages: `internal/adapters/scriptorium/runner.go`,
|
||||
Scriptorium adapter tests.
|
||||
- Duplicated or near-duplicated behavior: `Render` and `Run` both validate
|
||||
common inputs, resolve binary/runner defaults, execute a command, copy
|
||||
stdout/stderr/truncation/exit fields, and turn nonzero exit codes into
|
||||
result-plus-error. Argument construction is already separated.
|
||||
- Why it matters: future changes to capture limits, redaction, timeout handling,
|
||||
or nonzero exit formatting could drift between render and run.
|
||||
- Recommended refactor: introduce a private `execute` helper returning the
|
||||
shared command result fields and preserving command-specific validation and
|
||||
result structs.
|
||||
- Suggested tests: existing render/run argv and nonzero exit tests should remain
|
||||
sufficient; add a timeout/non-exit error test if one is missing.
|
||||
- Risk level: low-medium. Keep the public adapter API and result JSON stable.
|
||||
|
||||
### Consolidate storm time parsing and validation
|
||||
|
||||
- Affected files/packages: `internal/cli/root.go`, `internal/report/period.go`,
|
||||
CLI and report period tests.
|
||||
- Duplicated or near-duplicated behavior: CLI validates `--start`, `--end`,
|
||||
parses both storm timestamps, and checks end after start. Report resolution
|
||||
also validates storm start/end and has a `ParseStormPeriod` helper.
|
||||
- Why it matters: error wording and accepted timestamp behavior could drift
|
||||
between CLI and report-level validation.
|
||||
- Recommended refactor: keep CLI-specific missing-flag errors in `internal/cli`,
|
||||
but delegate parse/order validation to one helper after both values are
|
||||
present.
|
||||
- Suggested tests: keep CLI tests for missing flags and RFC3339 parsing; keep
|
||||
report period tests for invalid bounds.
|
||||
- Risk level: low.
|
||||
|
||||
### Add a briefing-level weather signal helper, but keep prose local
|
||||
|
||||
- Affected files/packages: `internal/briefing/daily.go`,
|
||||
`internal/briefing/three_day.go`, `internal/briefing/weekend.go`,
|
||||
`internal/briefing/storm.go`.
|
||||
- Duplicated or near-duplicated behavior: several builders aggregate ranges,
|
||||
max precipitation, peak gusts, hazards, alert events, and risk labels from
|
||||
daypart summaries. The report-specific prose and planning notes are
|
||||
intentionally different.
|
||||
- Why it matters: threshold changes for "wind", "precipitation", or hazard
|
||||
labels may need multiple edits.
|
||||
- Recommended refactor: add a narrow unexported helper in `internal/briefing`
|
||||
for aggregating common signals from dayparts and alerts. Do not centralize
|
||||
report-specific language, planning sections, or prompt-facing structure.
|
||||
- Suggested tests: briefing tests should assert unchanged daily, 3-day, weekend,
|
||||
and storm package shape for representative fixtures.
|
||||
- Risk level: medium. This is useful only if kept small; over-consolidating
|
||||
report prose would make the builders harder to read.
|
||||
|
||||
### Clean up legacy daily-specific wrappers after generic report support
|
||||
|
||||
- Affected files/packages: `internal/app/app.go`, `internal/state/store.go`,
|
||||
`internal/state/filesystem.go`, app and state tests.
|
||||
- Duplicated or near-duplicated behavior: `DailyBriefingRequest`,
|
||||
`DailyReportRequest`, `GenerateDailyBriefing`, `GenerateDailyReport`,
|
||||
`BuildDailyBriefing`, `FindPriorDailySnapshot`, and `dailyRecentChanges`
|
||||
mostly alias or forward to generic report functions.
|
||||
- Why it matters: wrappers create two names for the same behavior and encourage
|
||||
future code to depend on the older daily-specific path.
|
||||
- Recommended refactor: remove unused wrappers or mark them as test-only
|
||||
migration targets, then update tests to call the generic functions directly.
|
||||
Keep any wrapper that is intentionally part of a public package contract, but
|
||||
this repository uses `internal/`, so that concern is limited.
|
||||
- Suggested tests: app and state tests should continue to cover daily behavior
|
||||
through generic generation and prior snapshot paths.
|
||||
- Risk level: low-medium. This is a small internal API cleanup, but it touches
|
||||
many test call sites.
|
||||
|
||||
### Reduce duplicated end-to-end test setup
|
||||
|
||||
- Affected files/packages: `internal/cli/root_test.go`,
|
||||
`internal/app/app_test.go`, adapter tests.
|
||||
- Duplicated or near-duplicated behavior: tests repeatedly create temp
|
||||
workspaces, write YAML config strings, start representative Weather API
|
||||
servers, create fake Scriptorium scripts, and glob managed artifact paths.
|
||||
- Why it matters: setup duplication makes behavior-preserving refactors noisier
|
||||
and increases the chance that new tests accidentally use a subtly different
|
||||
fixture.
|
||||
- Recommended refactor: add small package-local helpers for config writing,
|
||||
test server setup, fake Scriptorium setup, and artifact glob/assertion.
|
||||
Avoid a cross-package test framework unless duplication becomes painful in
|
||||
more packages.
|
||||
- Suggested tests: this is test-only; existing tests should pass unchanged in
|
||||
behavior.
|
||||
- Risk level: low.
|
||||
|
||||
## 5. Boundary and responsibility concerns
|
||||
|
||||
The main boundary concern is that `internal/state` imports the Scriptorium
|
||||
adapter package and exposes `SavePreflight(context.Context, report.Resolved,
|
||||
*scriptorium.RenderResult)` through `state.Store`. State needs to persist a
|
||||
preflight artifact, but it does not need to know that the artifact type comes
|
||||
from an external CLI adapter. This is a small leak across the documented adapter
|
||||
boundary.
|
||||
|
||||
Recommended home: keep subprocess result construction in
|
||||
`internal/adapters/scriptorium`; have `internal/app` convert or pass the result
|
||||
to state through a state-owned preflight artifact type, a generic JSON artifact
|
||||
writer, or an app-owned persistence helper. This keeps adapter result types from
|
||||
becoming part of the state interface.
|
||||
|
||||
The second boundary concern is report policy in state/app. `internal/state`
|
||||
should own filesystem layout mechanics, but report grouping and compatible
|
||||
prior report selection are report catalog decisions. `internal/app` should
|
||||
orchestrate batch copies, but batch copy filename policy should come from report
|
||||
definitions.
|
||||
|
||||
The third concern is unused config surface. `internal/config` owns
|
||||
`ReportOutputConfig`, but current command behavior does not consume it. Either
|
||||
the app should use it explicitly or the config fields should be removed before
|
||||
they become accidental public API.
|
||||
|
||||
## 6. Path, key, and naming construction review
|
||||
|
||||
Local managed artifact paths are mostly centralized in
|
||||
`FilesystemStore.Paths`, which is good. That function owns the paths for
|
||||
briefing snapshots, metadata, data packages, preflight output, and managed
|
||||
Markdown reports.
|
||||
|
||||
Cleanup targets:
|
||||
|
||||
- `reportGroup` is private to `internal/state`, while report identity policy is
|
||||
canonical in `internal/report`.
|
||||
- Batch extra-copy names are built in `internal/app` by replacing underscores in
|
||||
`DefaultOutputName`. This derives a public filename by convention rather than
|
||||
declaring it.
|
||||
- Metadata path resolution uses `metadataPathFromStored`, derived from
|
||||
`BriefingPath`. This works today, but it means metadata path identity is
|
||||
partially reconstructed from another artifact path instead of coming directly
|
||||
from `ArtifactPaths`.
|
||||
- Tests in CLI/app/state hard-code path fragments in many places. These are
|
||||
useful regression checks, but after report group/name helpers exist, tests
|
||||
should assert through the helper or explicitly state they are path-contract
|
||||
tests.
|
||||
|
||||
No remote keys, cache paths, lock files, schema paths, or object-store paths are
|
||||
implemented.
|
||||
|
||||
## 7. Resolution and catalog review
|
||||
|
||||
Report resolution is mostly consistent. `internal/report` owns report
|
||||
definitions, prompt IDs, valid-period resolution, batches, and comparison
|
||||
strategy declarations.
|
||||
|
||||
Places that should be brought closer to the catalog:
|
||||
|
||||
- command-to-report mapping in `internal/app` (`ReportKind` to `report.ID`);
|
||||
- command-to-batch mapping in `internal/app`;
|
||||
- artifact group mapping in `internal/state`;
|
||||
- compatible prior report matching in `internal/state`;
|
||||
- batch copy filename behavior in `internal/app`;
|
||||
- "generated report" eligibility in `internal/app`.
|
||||
|
||||
Recommendation: keep the public CLI command names in `internal/cli`/`app`, but
|
||||
let the report registry expose enough metadata that app and state do not need
|
||||
parallel switches over report IDs.
|
||||
|
||||
Prompt and profile resolution is clean. Prompt IDs are declared in
|
||||
`internal/report`; Scriptorium profile/config/binary/extra args stay in config
|
||||
and the Scriptorium adapter. Data package naming is consistently
|
||||
`data_package=<path>`.
|
||||
|
||||
Weather API source resolution is explicit but somewhat repetitive. A source
|
||||
catalog or source spec table could help if the number of upstream sources grows.
|
||||
|
||||
## 8. Config and command-loading review
|
||||
|
||||
Configuration loading is centralized in `internal/config`, and the documented
|
||||
precedence is reflected in code: CLI overrides, config file, built-in defaults.
|
||||
There is no duplicated YAML parsing or validation outside `internal/config`.
|
||||
|
||||
Command loading is consistent but repetitive:
|
||||
|
||||
- generate commands load config with `--config`, `--units`, `--tz`, and `--out`;
|
||||
- run commands load config with `--config`, `--units`, and `--tz`;
|
||||
- inspect commands load config with `--config` only.
|
||||
|
||||
The inspect difference appears intentional because inspect commands do not
|
||||
fetch weather data or generate reports. The generate/run repetition is modest,
|
||||
but a small helper for building `config.LoadOptions` would reduce drift if more
|
||||
shared flags are added.
|
||||
|
||||
The likely accidental issue is `LoadOptions.Output`: it mutates
|
||||
`cfg.Reports.OutputDir`, but current generation behavior separately uses
|
||||
`GenerateRequest.OutputPath`, and docs say output flags do not change config.
|
||||
This should be removed or given real semantics.
|
||||
|
||||
## 9. State, manifest, or progress handling review
|
||||
|
||||
The project has filesystem state but no manifest, checkpoint, resume, force, or
|
||||
dry-run system.
|
||||
|
||||
State handling is generally consistent:
|
||||
|
||||
- `FilesystemStore.Paths` centralizes managed artifact paths.
|
||||
- Store methods persist briefing, data package, preflight, metadata, and managed
|
||||
report path preparation.
|
||||
- `GenerateReport` persists metadata before returning render errors when a
|
||||
render result exists.
|
||||
- `RunBatchDetailed` continues independent reports and returns aggregate
|
||||
results for CLI summarization.
|
||||
- Inspection loads metadata by RunID and then follows metadata paths.
|
||||
|
||||
Cleanup targets:
|
||||
|
||||
- State prior-snapshot lookup uses filesystem discovery plus hardcoded report
|
||||
compatibility. This is currently acceptable, but compatibility policy should
|
||||
be registry-driven before adding more report types.
|
||||
- `LoadMetadataByRunID` scans all reports through `ListReports`. This is simple
|
||||
and fine at current scale, but it is the place to revisit if workspace size
|
||||
grows.
|
||||
- `SaveMetadata` reconstructs the metadata path from `BriefingPath`, while most
|
||||
other artifact paths come from `Paths`. Prefer carrying the metadata path
|
||||
explicitly to reduce path coupling.
|
||||
|
||||
No progress tracking drift exists because progress tracking is not implemented.
|
||||
|
||||
## 10. Refactors to avoid
|
||||
|
||||
Avoid these refactors for now:
|
||||
|
||||
- A generic workflow engine for generation stages. The current explicit
|
||||
orchestration is readable and well tested.
|
||||
- A plugin architecture for reports, sources, validators, or adapters. The
|
||||
current registry and adapter packages are enough.
|
||||
- A broad CLI redesign or Cobra migration. The standard-library CLI is adequate
|
||||
and documented.
|
||||
- A sweeping manifest or resume-system rewrite. There is no implemented resume
|
||||
behavior to consolidate yet.
|
||||
- A generic source ingestion framework for the Weather API. A small helper or
|
||||
spec table is enough if new sources are added.
|
||||
- Consolidating all report briefing prose. Similar thresholds can be factored,
|
||||
but report-specific prompt inputs should remain readable and explicit.
|
||||
- Moving all test helpers into a global test package. Prefer package-local
|
||||
helpers until duplication crosses package boundaries in a way that blocks
|
||||
refactoring.
|
||||
- Removing path-contract tests merely because they duplicate strings. Some
|
||||
hard-coded expected paths are valuable regression coverage.
|
||||
|
||||
## 11. Recommended implementation sequence
|
||||
|
||||
1. Path/key/naming helpers
|
||||
- Add report registry metadata or helpers for artifact group, batch copy
|
||||
filename, generated-report eligibility, and compatible prior report IDs.
|
||||
- Preserve existing paths and filenames.
|
||||
- Update state/app tests first or in the same commit.
|
||||
|
||||
2. Atomic file and JSON helpers
|
||||
- Introduce a narrow atomic file helper.
|
||||
- Route state, briefing, prompt input, Weather API bundle saving,
|
||||
Scriptorium preflight saving, and report-copy code through it.
|
||||
- Remove unused save helpers if they no longer have callers.
|
||||
|
||||
3. Config loading context
|
||||
- Remove or implement `ReportOutputConfig` and `LoadOptions.Output`.
|
||||
- Keep documented CLI output behavior stable.
|
||||
- Add config regression tests around examples and CLI output flags.
|
||||
|
||||
4. Command preflight/shared CLI parsing
|
||||
- Add small helpers for common config-load options and inspect command
|
||||
dispatch.
|
||||
- Keep command errors and JSON output stable.
|
||||
|
||||
5. Shared state lookup helpers
|
||||
- Add app or state helpers for "load metadata by RunID, then load artifact".
|
||||
- Keep inspect output unchanged.
|
||||
|
||||
6. Adapter workflow cleanup
|
||||
- Factor Scriptorium shared execution internals.
|
||||
- Consider a Weather API optional-source helper only if source work is
|
||||
already planned.
|
||||
|
||||
7. Formatting/reporting cleanup
|
||||
- Collapse `writeRunSummary` and `writeJSON` into one JSON writer helper.
|
||||
- Leave stderr run logs as-is unless operator output requirements change.
|
||||
|
||||
8. Test helper and fixture cleanup
|
||||
- Add package-local helpers for repeated CLI/app setup.
|
||||
- Keep fixtures small and deterministic.
|
||||
|
||||
9. Dead-code/legacy sweep
|
||||
- Remove daily-specific wrapper aliases and `FindPriorDailySnapshot` after
|
||||
tests call generic paths.
|
||||
- Re-run full tests and update internal docs if any package contracts change.
|
||||
|
||||
## 12. Test strategy
|
||||
|
||||
Tests to run before refactoring:
|
||||
|
||||
- `go test ./internal/report ./internal/state`
|
||||
- `go test ./internal/app ./internal/cli`
|
||||
- `go test ./internal/config`
|
||||
- `go test ./internal/adapters/weatherapi ./internal/adapters/scriptorium`
|
||||
|
||||
Tests to add before or during cleanup:
|
||||
|
||||
- Report registry tests for artifact group, batch copy filename, generated
|
||||
eligibility, and compatible prior report IDs.
|
||||
- Atomic file helper tests for parent directory creation, overwrite behavior,
|
||||
and cleanup after write failure.
|
||||
- Config tests proving output flags do not mutate config, or tests defining the
|
||||
implemented behavior if `reports.output_dir` is kept.
|
||||
- CLI inspect table tests that cover `--config` parsing and missing RunID
|
||||
validation for each run-specific inspect command.
|
||||
- State tests that assert metadata path is taken from explicit artifact paths if
|
||||
`metadataPathFromStored` is removed.
|
||||
- Scriptorium adapter tests for shared nonzero exit and timeout handling after
|
||||
render/run execution is factored.
|
||||
|
||||
Tests that can accompany refactors:
|
||||
|
||||
- Weather API optional-source helper tests if source fetch boilerplate is
|
||||
factored.
|
||||
- Briefing fixture tests if common signal aggregation is factored.
|
||||
- Test-helper cleanup can rely on existing package tests if no production code
|
||||
changes.
|
||||
|
||||
## 13. Appendix: findings not worth acting on
|
||||
|
||||
- `internal/app.BuildBriefing` uses a switch over report IDs. This is acceptable
|
||||
because app orchestration needs to dispatch to report-specific builders. A
|
||||
registry of builder functions could be useful later, but it is not necessary
|
||||
for the current report count.
|
||||
- CLI command parsing is explicit and somewhat repetitive. Do not replace it
|
||||
with a framework. Small helpers are enough.
|
||||
- Daily, 3-day, weekend, and storm briefing builders have similar weather
|
||||
thresholds. Some signal aggregation can be shared, but report-specific
|
||||
sections and wording should stay local.
|
||||
- Weather API fetch methods are explicit. A helper is worthwhile only around
|
||||
repeated optional-source mechanics; endpoint-specific decode and timestamp
|
||||
behavior should remain clear.
|
||||
- Hard-coded expected artifact path fragments in tests duplicate path strings,
|
||||
but several of those tests intentionally protect the on-disk contract.
|
||||
- `LoadMetadataByRunID` scans filesystem metadata. That is acceptable at the
|
||||
current scale and should not be optimized without evidence of operational
|
||||
pain.
|
||||
- Batch stderr logs are simple text. Do not introduce a logging subsystem unless
|
||||
operator requirements become more complex.
|
||||
394
docs/roadmap/cleanup.md
Normal file
394
docs/roadmap/cleanup.md
Normal file
@@ -0,0 +1,394 @@
|
||||
# Cleanup Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
This roadmap defines the staged implementation plan for the code quality and
|
||||
deduplication cleanup identified in `docs/roadmap/audit.md`.
|
||||
|
||||
The target audience is an LLM coding agent. Implement each stage in order.
|
||||
Each stage should leave the repository buildable, tested, and behaviorally
|
||||
equivalent unless the stage explicitly removes unused public-facing surface.
|
||||
|
||||
## Cleanup Principles
|
||||
|
||||
- Preserve public CLI syntax and output behavior unless a stage explicitly says
|
||||
otherwise.
|
||||
- Preserve current managed artifact paths, report group names, RunID naming,
|
||||
and batch output copy filenames.
|
||||
- Keep domain policy in `internal/report`, `internal/forecast`,
|
||||
`internal/briefing`, and `internal/changes`, not in CLI or adapter packages.
|
||||
- Keep external system details behind adapter boundaries.
|
||||
- Prefer narrow, behavior-preserving helpers over broad framework-style
|
||||
abstractions.
|
||||
- Add or update focused tests in the package that owns the behavior being
|
||||
cleaned up.
|
||||
- Update non-roadmap documentation only after implemented behavior changes.
|
||||
- Do not revert unrelated worktree changes.
|
||||
|
||||
## Decisions Locked
|
||||
|
||||
- Add a narrow `internal/fileutil` package for reusable atomic file helpers.
|
||||
- Remove the unused `reports` config surface instead of implementing
|
||||
config-driven output defaults.
|
||||
- Move report artifact group, batch copy filename, generated-report eligibility,
|
||||
and compatible-prior policy into `internal/report`.
|
||||
- Keep public CLI command names and command-to-report mapping in `internal/app`
|
||||
for now.
|
||||
- Do not include Weather API source-spec refactoring in the main cleanup
|
||||
sequence.
|
||||
- Do not include broad briefing signal consolidation in the main cleanup
|
||||
sequence.
|
||||
- Do not introduce Cobra, a workflow engine, plugin system,
|
||||
manifest/resume/progress system, logging subsystem, or global test framework.
|
||||
- Treat the current unrelated deletion of `docs/roadmap/documentation.md` as
|
||||
out of scope for cleanup implementation. Do not restore or further modify it
|
||||
unless a later prompt explicitly asks for that.
|
||||
|
||||
## Stage 1: Report Catalog And Path Policy
|
||||
|
||||
Goal: make `internal/report` the canonical source for report identity policy.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Extend `report.Definition` with:
|
||||
- `ArtifactGroup string`
|
||||
- `BatchOutputName string`
|
||||
- `Generated bool`
|
||||
- `CompatiblePriorIDs []report.ID`
|
||||
- Populate current exact values:
|
||||
- Daily Today: artifact group `daily`, batch output `daily.md`, generated
|
||||
true, compatible with Daily Today and Daily Tomorrow.
|
||||
- Daily Tomorrow: artifact group `daily`, batch output `tomorrow.md`,
|
||||
generated true, compatible with Daily Today and Daily Tomorrow.
|
||||
- 3-Day: artifact group `three-day`, batch output `three-day.md`, generated
|
||||
true, compatible with 3-Day.
|
||||
- Weekend: artifact group `weekend`, batch output `weekend.md`, generated
|
||||
true, compatible with Weekend.
|
||||
- Storm: artifact group `storm`, batch output `storm.md`, generated true,
|
||||
compatible with Storm.
|
||||
- Add small methods or helpers in `internal/report` for compatibility checks if
|
||||
direct slice checks would duplicate logic in callers.
|
||||
- Update `internal/state` to use the resolved report definition's
|
||||
`ArtifactGroup` instead of private `reportGroup`.
|
||||
- Update prior snapshot lookup to use `CompatiblePriorIDs` instead of a
|
||||
state-owned compatibility switch.
|
||||
- Update `internal/app` to use `BatchOutputName` directly and remove
|
||||
underscore-to-hyphen derivation.
|
||||
- Update generated-report eligibility checks to use `Definition.Generated`.
|
||||
- Preserve all existing managed artifact paths and batch copy filenames.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add or update report registry tests that assert each definition's
|
||||
`ArtifactGroup`, `BatchOutputName`, `Generated`, and `CompatiblePriorIDs`.
|
||||
- Keep state path tests as path-contract tests and preserve their expected path
|
||||
strings.
|
||||
- Keep app batch output tests and preserve expected filenames such as
|
||||
`tomorrow.md`.
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/report ./internal/state ./internal/app
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- No report grouping switch remains in `internal/state`.
|
||||
- No batch output filename derivation by underscore replacement remains in
|
||||
`internal/app`.
|
||||
- Existing artifact paths and output copy names are unchanged.
|
||||
|
||||
## Stage 2: Atomic Artifact Writes And Adapter Boundary
|
||||
|
||||
Goal: centralize durable write mechanics and remove adapter type leakage from
|
||||
state.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Add `internal/fileutil` with:
|
||||
- `WriteFileAtomic(path string, data []byte) error`
|
||||
- `WriteJSONAtomic(path string, value any) error`
|
||||
- `CopyFileAtomic(source string, target string) error`
|
||||
- Implement helpers with the current behavior:
|
||||
- create parent directories with `0o755`;
|
||||
- create temp files in the target directory;
|
||||
- write, close, rename, and defer temp-file cleanup;
|
||||
- preserve useful path context in errors.
|
||||
- Route these through `internal/fileutil`:
|
||||
- state JSON writes;
|
||||
- briefing package save;
|
||||
- prompt input package save;
|
||||
- Weather API bundle save;
|
||||
- Scriptorium render result save if the helper remains;
|
||||
- generated report extra-copy writes.
|
||||
- Remove duplicate private atomic write helpers once callers are migrated.
|
||||
- Remove `scriptorium.SaveRenderResult` if no caller still needs it after the
|
||||
refactor.
|
||||
- Add `state.PreflightArtifact` with the same persisted JSON shape currently
|
||||
produced from `scriptorium.RenderResult`.
|
||||
- Change `state.Store.SavePreflight` to accept `state.PreflightArtifact`, not
|
||||
`*scriptorium.RenderResult`.
|
||||
- Convert `scriptorium.RenderResult` to `state.PreflightArtifact` in
|
||||
`internal/app` immediately before saving preflight output.
|
||||
- Keep subprocess result construction and interpretation in
|
||||
`internal/adapters/scriptorium`.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add `internal/fileutil` tests for parent directory creation, overwrite
|
||||
behavior, and cleanup/error behavior.
|
||||
- Keep state, app, adapter, briefing, and prompt input save tests.
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/fileutil ./internal/state ./internal/app ./internal/briefing ./internal/promptinput ./internal/adapters/weatherapi ./internal/adapters/scriptorium
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Durable JSON and Markdown copy writes share one implementation.
|
||||
- `internal/state` no longer imports `internal/adapters/scriptorium`.
|
||||
- Persisted preflight JSON remains shape-compatible with current artifacts.
|
||||
|
||||
## Stage 3: Remove Unused Report Output Config
|
||||
|
||||
Goal: remove config fields that do not affect implemented behavior.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Remove these unused config surfaces:
|
||||
- `Config.Reports`
|
||||
- `ReportOutputConfig`
|
||||
- `LoadOptions.Output`
|
||||
- default report output config values;
|
||||
- `reports.output_dir` validation;
|
||||
- `reports.paths` map initialization.
|
||||
- Update CLI generation config loading so `--out` remains only
|
||||
`GenerateRequest.OutputPath`.
|
||||
- Keep `--out` and `--out-dir` behavior unchanged.
|
||||
- Update config tests and examples so they mention only implemented config
|
||||
fields.
|
||||
- Update documentation in the same stage if non-roadmap docs still mention the
|
||||
removed `reports` config surface.
|
||||
|
||||
Tests:
|
||||
|
||||
- Update config tests to assert config loading no longer has output-related
|
||||
config behavior.
|
||||
- Keep CLI tests for `--out` and `--out-dir`.
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/config ./internal/cli ./internal/app
|
||||
go run ./cmd/weatherreporter --help
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- No `ReportOutputConfig` or `LoadOptions.Output` symbols remain.
|
||||
- Example config files load through existing config tests.
|
||||
- CLI output-copy behavior remains command request behavior, not config behavior.
|
||||
|
||||
## Stage 4: Inspect Command And State Lookup Cleanup
|
||||
|
||||
Goal: remove repeated inspect scaffolding while preserving inspect output.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Add a small inspect command table in `internal/cli`.
|
||||
- Keep `inspect reports` separate because it accepts `--limit` and does not
|
||||
require a RunID.
|
||||
- For run-specific inspect commands, centralize:
|
||||
- command name;
|
||||
- flag parsing;
|
||||
- config loading;
|
||||
- app handler invocation;
|
||||
- JSON output writing.
|
||||
- In `internal/app`, add an unexported helper that loads the default store and
|
||||
metadata for a RunID.
|
||||
- Use the app helper for metadata, briefing, data-package, prior, and sources
|
||||
inspection.
|
||||
- Collapse `writeRunSummary` and `writeJSON` into one JSON writer helper.
|
||||
- Preserve current JSON indentation and output shapes.
|
||||
|
||||
Tests:
|
||||
|
||||
- Keep `TestRunInspectGeneratedArtifacts`.
|
||||
- Keep missing metadata tests.
|
||||
- Add a table test proving run-specific inspect commands reject missing RunID
|
||||
and accept `--config`.
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/cli ./internal/app ./internal/state
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Inspect command output is unchanged.
|
||||
- Config loading remains consistent across inspect commands.
|
||||
- Repeated store and metadata lookup code in app inspection paths is removed.
|
||||
|
||||
## Stage 5: Small Adapter And Parser Deduplication
|
||||
|
||||
Goal: reduce low-risk repeated validation and execution logic.
|
||||
|
||||
Implementation:
|
||||
|
||||
- In `internal/adapters/scriptorium`, add a private execution helper shared by
|
||||
`Render` and `Run`.
|
||||
- Keep command-specific request validation and result structs.
|
||||
- Preserve:
|
||||
- argv order;
|
||||
- result JSON fields;
|
||||
- stdout/stderr capture;
|
||||
- truncation fields;
|
||||
- timeout behavior;
|
||||
- nonzero exit behavior and error text.
|
||||
- In storm CLI parsing, keep CLI-specific missing `--start` and `--end` errors.
|
||||
- After both storm bounds are present, delegate timestamp parsing and
|
||||
end-after-start validation to `report.ParseStormPeriod`.
|
||||
- Do not introduce a generic Weather API ingestion framework in this stage.
|
||||
|
||||
Tests:
|
||||
|
||||
- Keep or add tests for Scriptorium render/run nonzero exits.
|
||||
- Keep storm tests for local timestamps, RFC3339 timestamps, missing flags, and
|
||||
invalid bounds.
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/adapters/scriptorium ./internal/cli ./internal/report
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Scriptorium render/run behavior remains byte-for-byte compatible where tests
|
||||
assert argv or output shape.
|
||||
- Storm accepted timestamp formats and error behavior remain stable.
|
||||
|
||||
## Stage 6: Legacy Wrapper And Test Helper Cleanup
|
||||
|
||||
Goal: remove stale generic-vs-daily duplication and reduce noisy test setup.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Replace tests and internal callers of:
|
||||
- `GenerateDailyBriefing`
|
||||
- `GenerateDailyReport`
|
||||
- `BuildDailyBriefing`
|
||||
- `DailyBriefingRequest`
|
||||
- `DailyReportRequest`
|
||||
- Use generic functions and types instead:
|
||||
- `GenerateBriefing`
|
||||
- `GenerateReport`
|
||||
- `BuildBriefing`
|
||||
- `BriefingRequest`
|
||||
- `ReportRequest`
|
||||
- Remove `FindPriorDailySnapshot` from `state.Store` and
|
||||
`FilesystemStore` after tests use `FindPriorSnapshot`.
|
||||
- Remove `dailyRecentChanges` if no callers remain.
|
||||
- Add package-local test helpers in `internal/cli` and `internal/app` for:
|
||||
- writing test config files;
|
||||
- fake Scriptorium setup;
|
||||
- representative Weather API test server setup;
|
||||
- artifact glob and assertion helpers.
|
||||
- Do not create a cross-package test framework.
|
||||
|
||||
Tests:
|
||||
|
||||
- Run:
|
||||
|
||||
```bash
|
||||
go test ./internal/app ./internal/state ./internal/cli
|
||||
go test ./internal/...
|
||||
```
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Daily-specific app/state wrapper symbols listed above are gone.
|
||||
- Daily behavior remains covered through generic report-generation paths.
|
||||
- Test helper extraction does not reduce workflow coverage.
|
||||
|
||||
## Stage 7: Documentation And Final Validation
|
||||
|
||||
Goal: align implemented documentation after cleanup.
|
||||
|
||||
Implementation:
|
||||
|
||||
- Update non-roadmap docs only for behavior or internal contracts actually
|
||||
changed by stages 1 through 6.
|
||||
- Inspect and update, as needed:
|
||||
- `docs/config.md`
|
||||
- `docs/internal/state.md`
|
||||
- `docs/internal/scriptorium-adapter.md`
|
||||
- `docs/internal/report-registry.md`
|
||||
- `docs/policy/development.md`
|
||||
- relevant files under `docs/integrations/`
|
||||
- Keep future or deferred cleanup ideas only under `docs/roadmap/`.
|
||||
- Do not document deferred Weather API source-spec or briefing signal refactors
|
||||
as implemented.
|
||||
|
||||
Validation:
|
||||
|
||||
```bash
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Run stale-symbol searches:
|
||||
|
||||
```bash
|
||||
rg -n "ReportOutputConfig|LoadOptions\\.Output|FindPriorDailySnapshot|GenerateDailyReport|DailyReportRequest|SaveRenderResult" .
|
||||
```
|
||||
|
||||
Review any matches manually. Matches under roadmap files may be acceptable
|
||||
because they describe planned or completed cleanup work.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Non-roadmap docs describe only implemented behavior.
|
||||
- Config examples still load through config tests.
|
||||
- CLI help remains accurate.
|
||||
- No removed production symbols remain outside tests or roadmap references.
|
||||
|
||||
## Deferred Refactors
|
||||
|
||||
Do not include these in the main cleanup sequence:
|
||||
|
||||
- Weather API optional-source spec/helper refactor.
|
||||
- Broad briefing weather-signal consolidation.
|
||||
- Generic workflow engine.
|
||||
- Plugin architecture.
|
||||
- Cobra migration.
|
||||
- Manifest/resume/progress system.
|
||||
- Global test helper package.
|
||||
- Logging subsystem.
|
||||
|
||||
These can be revisited only when new source types, report types, or operational
|
||||
requirements make the duplication materially more expensive.
|
||||
|
||||
## Global Validation Checklist
|
||||
|
||||
Run focused tests after each stage, then run full validation after Stage 7.
|
||||
|
||||
Required final checks:
|
||||
|
||||
```bash
|
||||
go test ./...
|
||||
go run ./cmd/weatherreporter --help
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Required manual checks:
|
||||
|
||||
- Public CLI syntax remains stable.
|
||||
- Managed artifact paths remain stable.
|
||||
- Batch output copy filenames remain stable.
|
||||
- `scriptorium` argv construction remains stable.
|
||||
- Weather API request query behavior remains stable.
|
||||
- Removed config fields are also removed from current-behavior docs and
|
||||
examples.
|
||||
- Roadmap files are the only docs that describe deferred cleanup work.
|
||||
- No unrelated worktree changes are reverted.
|
||||
@@ -1,794 +0,0 @@
|
||||
# Documentation Roadmap
|
||||
|
||||
## Purpose
|
||||
|
||||
This roadmap defines the work required to bring `weatherreporter` documentation
|
||||
into compliance with `docs/policy/documentation.md` and the current
|
||||
implementation.
|
||||
|
||||
The migration must keep current-behavior documentation limited to implemented
|
||||
behavior. Planned, future, aspirational, experimental, deprecated, or
|
||||
unimplemented work belongs only under `docs/roadmap/`.
|
||||
|
||||
## Repository Documentation Inventory
|
||||
|
||||
- `README.md` - keep and lightly update. It already provides project purpose,
|
||||
quickstart commands, and documentation links, but it should remain short and
|
||||
should not link to stale implementation-roadmap material as though it were
|
||||
current documentation.
|
||||
- `docs/cli.md` - keep and lightly update. It is the canonical CLI reference
|
||||
and should be checked against `internal/cli/root.go` and
|
||||
`internal/cli/root_test.go`.
|
||||
- `docs/config.md` - keep and lightly update. It is the canonical
|
||||
configuration reference and should be checked against `internal/config/*`,
|
||||
`examples/config.yml`, and config tests.
|
||||
- `docs/operations.md` - keep and lightly update. It documents implemented
|
||||
generation, batch, inspection, artifact, metadata, and recovery behavior.
|
||||
Some recurring failure-mode content should move or link to the new
|
||||
troubleshooting guide.
|
||||
- `docs/troubleshooting.md` - create new. The project has enough implemented
|
||||
CLI, config, state, Weather API, and Scriptorium failure modes to warrant a
|
||||
focused troubleshooting guide.
|
||||
- `docs/internal/briefing.md` - keep and standardize. It documents an
|
||||
implemented component, but it should be aligned with the full internal-doc
|
||||
field list required by the documentation policy.
|
||||
- `docs/internal/changes.md` - keep and standardize. It documents implemented
|
||||
recent-change comparison and should remain scoped to structured snapshots,
|
||||
implemented report compatibility, and current thresholds.
|
||||
- `docs/internal/forecast-derivation.md` - keep and standardize. It documents
|
||||
implemented forecast derivation and should explicitly identify config fields,
|
||||
adapter inputs, state behavior, and invariants.
|
||||
- `docs/internal/prompt-input.md` - keep and standardize. It documents the
|
||||
implemented prompt input package and should stay focused on
|
||||
`internal/promptinput`.
|
||||
- `docs/internal/report-registry.md` - keep and standardize. It documents the
|
||||
implemented registry and valid-period behavior.
|
||||
- `docs/internal/scriptorium-adapter.md` - keep and standardize. It documents
|
||||
the implemented subprocess adapter and should link to
|
||||
`docs/integrations/scriptorium.md` for external contract details.
|
||||
- `docs/internal/state.md` - keep and standardize. It documents implemented
|
||||
filesystem state and should remain canonical for internal state behavior.
|
||||
- `docs/internal/weather-data.md` - keep and standardize. It documents the
|
||||
implemented Weather API adapter and forecast bundle boundary.
|
||||
- `docs/internal/app-orchestration.md` - create new. `internal/app` is the
|
||||
implemented workflow coordinator and needs its own internal component doc.
|
||||
- `docs/integrations/scriptorium.md` - keep and lightly update. It documents an
|
||||
actual external CLI integration and should stay limited to the commands,
|
||||
arguments, inputs, outputs, and exit behavior used by `weatherreporter`.
|
||||
- `docs/integrations/weatherapi.md` - keep and rewrite. It currently describes
|
||||
more Weather API surface than `weatherreporter` uses; narrow it to the
|
||||
implemented fan-out endpoints, envelope conventions, query parameters, and
|
||||
source warning/provenance expectations.
|
||||
- `docs/policy/architecture.md` - keep and lightly update only if accuracy
|
||||
issues are found. It is the canonical architecture policy.
|
||||
- `docs/policy/development.md` - keep and rewrite. It currently reads like a
|
||||
proposed package layout and includes future/MVP language. It should become
|
||||
the canonical contributor workflow document.
|
||||
- `docs/policy/documentation.md` - keep. It is the controlling documentation
|
||||
policy for this migration.
|
||||
- `docs/roadmap/future.md` - keep as the future-only project roadmap. The stale
|
||||
implementation roadmap was removed after deferred work was extracted.
|
||||
- `examples/config.yml` - keep and lightly update. It is a maintained
|
||||
production-oriented example config and should be validated against the
|
||||
implemented config loader.
|
||||
- `examples/minimal-config.yml` - create new only if the implementation pass
|
||||
adds a validation path for it. It should contain the smallest useful config
|
||||
for implemented generation behavior.
|
||||
|
||||
## Policy Compliance Assessment
|
||||
|
||||
Required documents for a modular, CLI/config-driven, stateful application are
|
||||
present: `README.md`, `docs/cli.md`, `docs/config.md`,
|
||||
`docs/operations.md`, `docs/internal/`, and `docs/policy/development.md`.
|
||||
|
||||
Recommended documentation is also present:
|
||||
|
||||
- `docs/troubleshooting.md` covers recurring operator-facing failure modes.
|
||||
- `examples/config.yml` and `examples/minimal-config.yml` are validated by
|
||||
config tests.
|
||||
- `docs/internal/app-orchestration.md` documents the workflow coordinator.
|
||||
|
||||
Documents that were stale or in the wrong canonical home have been corrected:
|
||||
|
||||
- `docs/policy/development.md` is the contributor workflow policy.
|
||||
- `docs/roadmap/future.md` is the current home for deferred project work.
|
||||
- `README.md` links to current user/operator/developer docs.
|
||||
- `docs/integrations/weatherapi.md` is limited to the implemented adapter
|
||||
contract.
|
||||
|
||||
Content that appears planned, historical, or aspirational outside
|
||||
`docs/roadmap/`:
|
||||
|
||||
- Non-roadmap docs should remain limited to implemented behavior. Some
|
||||
policy-level wording about future work is legitimate; feature-specific
|
||||
deferred behavior belongs under `docs/roadmap/`.
|
||||
|
||||
Examples:
|
||||
|
||||
- `examples/config.yml` and `examples/minimal-config.yml` exist and load
|
||||
through the config test suite.
|
||||
- No generated report examples should be added unless they can be maintained
|
||||
without live Weather API and Scriptorium dependencies.
|
||||
- No workflow examples should be added for unimplemented daemon, cleanup,
|
||||
archive, remote storage, or automatic storm monitoring behavior.
|
||||
|
||||
Links likely needing verification:
|
||||
|
||||
- README should link only to current user/operator/developer docs unless a
|
||||
clearly labeled future-work link is needed.
|
||||
- Internal docs should link to canonical integration docs instead of repeating
|
||||
Scriptorium or Weather API details.
|
||||
- Operations and troubleshooting should link to CLI and config reference rather
|
||||
than duplicating complete flag or field tables.
|
||||
|
||||
## Target Documentation Set
|
||||
|
||||
### `README.md`
|
||||
|
||||
- Audience: users, administrators, operators.
|
||||
- Purpose: concise orientation and shortest useful generation command.
|
||||
- Canonical scope: project purpose, elevator pitch, one minimal usage example,
|
||||
and links to targeted docs.
|
||||
- Recommended outline: description, elevator pitch, shortest useful command,
|
||||
documentation links.
|
||||
- Source-of-truth areas: `cmd/weatherreporter/main.go`,
|
||||
`internal/cli/root.go`, `internal/app/app.go`, `docs/cli.md`.
|
||||
- Acceptance criteria: short enough to remain an orientation page; no
|
||||
unimplemented behavior; no stale roadmap link presented as current docs;
|
||||
commands match `weatherreporter --help`.
|
||||
|
||||
### `docs/cli.md`
|
||||
|
||||
- Audience: users, administrators, operators.
|
||||
- Purpose: canonical CLI reference.
|
||||
- Canonical scope: implemented commands, flags, workflows, diagnostics, and
|
||||
recovery-oriented inspect commands.
|
||||
- Recommended outline: shortest useful command, command overview, flag
|
||||
reference, common workflows, inspection commands, exit behavior.
|
||||
- Source-of-truth areas: `internal/cli/root.go`,
|
||||
`internal/cli/root_test.go`, `internal/app/app.go`,
|
||||
`internal/app/app_test.go`.
|
||||
- Acceptance criteria: every documented command exists; every documented flag is
|
||||
parsed; `generate daily --date` is optional and documented as `YYYY-MM-DD`;
|
||||
batch nonzero behavior is documented; no unimplemented commands appear.
|
||||
|
||||
### `docs/config.md`
|
||||
|
||||
- Audience: administrators, operators, advanced users.
|
||||
- Purpose: canonical configuration reference.
|
||||
- Canonical scope: config discovery, precedence, minimal config, production
|
||||
config, full field reference, and examples.
|
||||
- Recommended outline: file location and precedence, minimal config, production
|
||||
config, field reference, secrets handling, links to examples.
|
||||
- Source-of-truth areas: `internal/config/config.go`,
|
||||
`internal/config/defaults.go`, `internal/config/load.go`,
|
||||
`internal/config/validate.go`, `internal/config/config_test.go`,
|
||||
`examples/config.yml`.
|
||||
- Acceptance criteria: defaults match code; CLI overrides are accurate;
|
||||
`weather_api.base_url` is described as required for generation/fetching;
|
||||
no unused or future config fields appear.
|
||||
|
||||
### `docs/operations.md`
|
||||
|
||||
- Audience: administrators, operators.
|
||||
- Purpose: operational workflows, state layout, inspection, recovery, and caveats.
|
||||
- Canonical scope: implemented generation and batch operation, workspace layout,
|
||||
metadata, artifact inspection, retry/recovery behavior, and explicit
|
||||
non-behavior where operationally important.
|
||||
- Recommended outline: normal workflow, filesystem layout, RunID and metadata,
|
||||
inspection, recent changes, recovery, current operational caveats.
|
||||
- Source-of-truth areas: `internal/app/app.go`, `internal/app/inspect.go`,
|
||||
`internal/state/*`, `internal/report/*`, `internal/cli/root.go`.
|
||||
- Acceptance criteria: workspace paths match `internal/state`; batch behavior
|
||||
matches `RunBatchDetailed`; recovery guidance reflects actual persisted
|
||||
artifacts; troubleshooting details are linked rather than duplicated.
|
||||
|
||||
### `docs/troubleshooting.md`
|
||||
|
||||
- Audience: administrators, operators.
|
||||
- Purpose: symptom-driven diagnosis and safe fixes.
|
||||
- Canonical scope: recurring failure modes for config loading, CLI parsing,
|
||||
Weather API fetching, missing data policy, Scriptorium render/run, workspace
|
||||
state, and inspect commands.
|
||||
- Recommended outline: symptom, likely cause, diagnostic command or inspection
|
||||
step, safe fix, relevant links.
|
||||
- Source-of-truth areas: `internal/cli/root.go`, `internal/config/*`,
|
||||
`internal/adapters/weatherapi/*`, `internal/adapters/scriptorium/*`,
|
||||
`internal/state/*`, tests for those packages.
|
||||
- Acceptance criteria: entries are actionable; no speculative failures; links
|
||||
point to CLI/config/operations/integration docs; no secrets or private
|
||||
infrastructure examples.
|
||||
|
||||
### `docs/policy/architecture.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: canonical architecture policy and invariants.
|
||||
- Canonical scope: project shape, boundaries, dependency policy, configuration,
|
||||
adapters, state, testing, and documentation expectations.
|
||||
- Recommended outline: keep current structure unless implementation inspection
|
||||
reveals an inaccurate invariant.
|
||||
- Source-of-truth areas: current package layout and tests.
|
||||
- Acceptance criteria: policy remains principle-level; it does not become a
|
||||
package manual; current implementation does not obviously violate described
|
||||
invariants.
|
||||
|
||||
### `docs/policy/development.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: canonical contributor workflow.
|
||||
- Canonical scope: repository layout, build/test commands, coding conventions,
|
||||
dependency policy, adding config fields, adding CLI flags, adding components
|
||||
and adapters, updating examples, and documentation expectations.
|
||||
- Recommended outline: repository layout, local validation commands, coding
|
||||
conventions, dependencies, config changes, CLI changes, component/adapters,
|
||||
tests, docs/examples checklist.
|
||||
- Source-of-truth areas: `go.mod`, package layout, `internal/config`,
|
||||
`internal/cli`, adapters, tests, `docs/policy/documentation.md`.
|
||||
- Acceptance criteria: no MVP/proposed/future feature narrative; no duplicate
|
||||
full architecture manual; future work appears only as links to roadmap docs.
|
||||
|
||||
### `docs/internal/app-orchestration.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: describe implemented workflow orchestration in `internal/app`.
|
||||
- Canonical scope: generation flow, batch flow, inspection flow, dependencies,
|
||||
state writes, preflight/run ordering, and failure behavior.
|
||||
- Recommended outline: purpose, inputs and outputs, boundaries, config fields
|
||||
used, adapters used, state behavior, skip/resume behavior, failure behavior,
|
||||
tests, invariants.
|
||||
- Source-of-truth areas: `internal/app/app.go`, `internal/app/inspect.go`,
|
||||
`internal/app/app_test.go`, `internal/state/*`.
|
||||
- Acceptance criteria: documents `scriptorium render` before `scriptorium run`;
|
||||
documents metadata persistence before/after generation failures; does not
|
||||
expose adapter internals beyond orchestration needs.
|
||||
|
||||
### `docs/internal/briefing.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: implemented briefing package and report-specific briefing builders.
|
||||
- Canonical scope: briefing inputs, outputs, boundaries, source warnings,
|
||||
metadata, report variants, tests, and invariants.
|
||||
- Source-of-truth areas: `internal/briefing/*`, `internal/forecast/*`,
|
||||
briefing tests.
|
||||
- Acceptance criteria: includes all internal-doc policy fields; no raw API
|
||||
endpoint documentation duplicated from integrations.
|
||||
|
||||
### `docs/internal/changes.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: structured recent-change comparison.
|
||||
- Canonical scope: comparable snapshot inputs, thresholds, supported report
|
||||
strategies, output items, and empty-result behavior.
|
||||
- Source-of-truth areas: `internal/changes/*`, `internal/report/*`,
|
||||
`internal/state/*`, changes tests.
|
||||
- Acceptance criteria: states that comparisons use structured snapshots, not
|
||||
rendered Markdown; storm current behavior is accurate.
|
||||
|
||||
### `docs/internal/forecast-derivation.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: forecast derivation from normalized bundle data.
|
||||
- Canonical scope: hourly requirement, dayparts, daily/period summaries,
|
||||
alerts, narrative/discussion usage, warnings, and failure behavior.
|
||||
- Source-of-truth areas: `internal/forecast/*`, forecast tests and testdata.
|
||||
- Acceptance criteria: config fields and adapter inputs are explicit; hourly
|
||||
failure behavior matches tests.
|
||||
|
||||
### `docs/internal/prompt-input.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: prompt input data package construction.
|
||||
- Canonical scope: package schema, report metadata, briefing, recent changes,
|
||||
source warnings, validation, and boundaries.
|
||||
- Source-of-truth areas: `internal/promptinput/*`, `internal/app/app.go`,
|
||||
prompt input tests.
|
||||
- Acceptance criteria: uses `data_package` terminology; no `promptvars` or
|
||||
`--vars-file` terminology.
|
||||
|
||||
### `docs/internal/report-registry.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: report registry, prompt IDs, valid periods, batches, and comparison
|
||||
strategy declarations.
|
||||
- Canonical scope: implemented report IDs, prompt IDs, default output naming,
|
||||
period resolution, batch composition, and compatibility behavior.
|
||||
- Source-of-truth areas: `internal/report/*`, report tests,
|
||||
`internal/app/app.go`.
|
||||
- Acceptance criteria: documents `weather.daily_report` for both Daily Today
|
||||
and Daily Tomorrow; no stale `weather.tomorrow_report`.
|
||||
|
||||
### `docs/internal/scriptorium-adapter.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: internal subprocess adapter boundary.
|
||||
- Canonical scope: request structs, argv construction, context/timeouts,
|
||||
stdout/stderr capture, result handling, and errors.
|
||||
- Source-of-truth areas: `internal/adapters/scriptorium/*`,
|
||||
scriptorium adapter tests.
|
||||
- Acceptance criteria: no shell interpolation; domain packages do not receive
|
||||
Scriptorium-specific flags; links to integration contract for CLI details.
|
||||
|
||||
### `docs/internal/state.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: filesystem store, metadata, artifact paths, and lookup behavior.
|
||||
- Canonical scope: workspace layout, RunID-managed files, metadata records,
|
||||
prior snapshot lookup, inspection, atomic write expectations, failure cases.
|
||||
- Source-of-truth areas: `internal/state/*`, state tests,
|
||||
`internal/app/app.go`.
|
||||
- Acceptance criteria: paths match code; no resume/archive/cleanup behavior is
|
||||
documented as implemented.
|
||||
|
||||
### `docs/internal/weather-data.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: Weather API adapter to internal forecast bundle.
|
||||
- Canonical scope: implemented endpoint fan-out, required vs optional sources,
|
||||
missing-source policy, source hashes, provenance, and warnings.
|
||||
- Source-of-truth areas: `internal/adapters/weatherapi/*`,
|
||||
`internal/forecast/bundle.go`, weather adapter tests and testdata.
|
||||
- Acceptance criteria: documents hourly as required; optional/stub sources
|
||||
match code; links to Weather API integration doc for external contract.
|
||||
|
||||
### `docs/integrations/scriptorium.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: external Scriptorium CLI contract used by `weatherreporter`.
|
||||
- Canonical scope: `scriptorium render`, `scriptorium run`,
|
||||
`--input data_package=<path>`, optional config/profile/extra args,
|
||||
stdout/stderr, output path, and exit behavior.
|
||||
- Source-of-truth areas: `internal/adapters/scriptorium/runner.go`,
|
||||
`internal/adapters/scriptorium/runner_test.go`, `internal/config/*`.
|
||||
- Acceptance criteria: only implemented invocation shapes are documented; no
|
||||
unused Scriptorium features appear as project behavior.
|
||||
|
||||
### `docs/integrations/weatherapi.md`
|
||||
|
||||
- Audience: developers and LLM coding agents.
|
||||
- Purpose: external Weather API contract used by `weatherreporter`.
|
||||
- Canonical scope: base URL handling, response envelope, query parameters,
|
||||
implemented endpoints, source identity, timestamps, missing data, and
|
||||
compatibility assumptions.
|
||||
- Source-of-truth areas: `internal/adapters/weatherapi/client.go`,
|
||||
`internal/adapters/weatherapi/client_test.go`, adapter testdata.
|
||||
- Acceptance criteria: documents `/observations`, `/conditions/current`,
|
||||
`/forecast/hourly`, `/forecast/narrative`, `/alerts/active`, and
|
||||
`/discussion`; does not present unused day-slice endpoints as current usage.
|
||||
|
||||
### `docs/roadmap/documentation.md`
|
||||
|
||||
- Audience: maintainers, developers, LLM coding agents.
|
||||
- Purpose: this migration plan.
|
||||
- Canonical scope: planned documentation migration only.
|
||||
- Source-of-truth areas: documentation policy and current repository.
|
||||
- Acceptance criteria: action-oriented; distinguishes required and recommended
|
||||
work; does not rewrite current docs inline; includes validation commands.
|
||||
|
||||
### `docs/roadmap/future.md`
|
||||
|
||||
- Audience: maintainers, developers, LLM coding agents.
|
||||
- Purpose: future-only project work extracted from stale roadmap material.
|
||||
- Canonical scope: deferred features such as automatic storm monitoring, if
|
||||
still desired.
|
||||
- Source-of-truth areas: current code boundaries and deferred work already
|
||||
extracted into this file.
|
||||
- Acceptance criteria: no completed MVP tasks; no claims of current behavior;
|
||||
each item is clearly labeled proposed, accepted, deferred, or rejected.
|
||||
|
||||
### `examples/config.yml`
|
||||
|
||||
- Audience: administrators, operators, developers.
|
||||
- Purpose: production-oriented example configuration.
|
||||
- Canonical scope: implemented config fields only, no secrets.
|
||||
- Source-of-truth areas: `internal/config/*`, `internal/config/config_test.go`.
|
||||
- Acceptance criteria: loads successfully; linked from `docs/config.md`; no
|
||||
private endpoint or credential values.
|
||||
|
||||
### `examples/minimal-config.yml`
|
||||
|
||||
- Audience: administrators, operators, developers.
|
||||
- Purpose: smallest useful config for implemented generation behavior.
|
||||
- Canonical scope: only required or commonly changed fields.
|
||||
- Source-of-truth areas: `internal/config/*`, config tests.
|
||||
- Acceptance criteria: add only with validation coverage; linked from
|
||||
`docs/config.md`; no unimplemented fields.
|
||||
|
||||
## File-by-File Rewrite Guidance
|
||||
|
||||
- `README.md`: keep it short. Cover what the application does, one shortest
|
||||
useful command, and links to canonical docs. Avoid manual-level flag tables,
|
||||
state layout details, implementation stages, or stale roadmap links.
|
||||
- `docs/cli.md`: inspect `internal/cli/root.go` and CLI tests before editing.
|
||||
Cover real commands and flags only. Link to config for config fields and to
|
||||
operations for artifact/state behavior.
|
||||
- `docs/config.md`: inspect `internal/config` and `examples/config.yml`.
|
||||
Document precedence and defaults in one place. Avoid repeating operations or
|
||||
CLI workflow material.
|
||||
- `docs/operations.md`: inspect `internal/app`, `internal/state`, and
|
||||
`internal/report`. Cover normal operation, artifacts, metadata, inspect
|
||||
commands, batch behavior, and recovery. Avoid detailed troubleshooting
|
||||
entries that belong in `docs/troubleshooting.md`.
|
||||
- `docs/troubleshooting.md`: create symptom-driven entries only for implemented
|
||||
failure modes. Likely entries include missing `weather_api.base_url`, invalid
|
||||
timezone, invalid storm bounds, Weather API missing hourly data, optional
|
||||
source warnings, `scriptorium` not found, render/run nonzero exit, missing
|
||||
workspace artifacts, and unknown RunID.
|
||||
- `docs/policy/development.md`: rewrite as contributor workflow. Do not carry
|
||||
forward "proposed layout", "MVP should", "future backend", or automatic storm
|
||||
monitoring guidance except as roadmap links.
|
||||
- `docs/policy/architecture.md`: edit only if a current invariant is wrong.
|
||||
Keep it principle-level and avoid duplicating internal component docs.
|
||||
- `docs/internal/*`: standardize each implemented component doc to the policy
|
||||
field list. Avoid documenting unimplemented resume, cleanup, archive, remote
|
||||
storage, daemon, or automatic storm-monitoring behavior.
|
||||
- `docs/integrations/weatherapi.md`: narrow the endpoint section to actual
|
||||
adapter calls. Document source hash behavior as SHA-256 over canonical,
|
||||
minified raw `data` JSON. Do not carry forward unused endpoint examples as
|
||||
current project behavior.
|
||||
- `docs/integrations/scriptorium.md`: keep `--input data_package=<path>` as the
|
||||
documented input contract. Do not reintroduce `--vars-file` or `promptvars`.
|
||||
- `docs/roadmap/future.md`: keep only deferred work and avoid completed
|
||||
implementation history.
|
||||
- `examples/config.yml`: keep as production-oriented config. Validate it with
|
||||
config-loading tests or an equivalent non-secret check.
|
||||
- `examples/minimal-config.yml`: add only if the implementation agent also adds
|
||||
a validation check and can point `docs/config.md` at it.
|
||||
|
||||
## Examples Plan
|
||||
|
||||
`examples/` currently exists and contains `examples/config.yml`.
|
||||
|
||||
Recommended examples:
|
||||
|
||||
- `examples/config.yml`
|
||||
- Purpose: production-oriented config covering implemented fields.
|
||||
- Expected validity check: load through `internal/config` in tests or an
|
||||
equivalent config validation command.
|
||||
- Documentation links: `docs/config.md`; optional link from README only if the
|
||||
README remains concise.
|
||||
- `examples/minimal-config.yml`
|
||||
- Purpose: smallest useful config for generation against a configured Weather
|
||||
API and Scriptorium installation.
|
||||
- Expected validity check: add test coverage that loads the file and confirms
|
||||
defaults fill omitted fields.
|
||||
- Documentation links: `docs/config.md`.
|
||||
|
||||
Do not add generated report examples in this migration. They depend on live
|
||||
Weather API and Scriptorium behavior and would become stale unless fixture-based
|
||||
generation is implemented.
|
||||
|
||||
Do not add examples for automatic storm monitoring, daemon operation, cleanup,
|
||||
archive, remote storage, or resume behavior because those are not implemented.
|
||||
|
||||
## Internal Documentation Plan
|
||||
|
||||
- App orchestration
|
||||
- Path: `docs/internal/app-orchestration.md`
|
||||
- Purpose: explain implemented generation, batch, inspection, preflight, run,
|
||||
and persistence ordering.
|
||||
- Inputs and outputs: app requests, config, report definitions, forecast
|
||||
bundle, briefing, data package, metadata, preflight result, rendered report,
|
||||
batch summary.
|
||||
- Boundaries: coordinates packages but does not own forecast derivation,
|
||||
config parsing, adapter internals, or report definitions.
|
||||
- Config fields used: Weather API, Scriptorium, workspace, report output,
|
||||
dayparts, recent-change thresholds.
|
||||
- Adapters used: Weather API and Scriptorium.
|
||||
- Failure behavior: render failures persist preflight and metadata when
|
||||
available; run failures leave inspectable artifacts; batches continue
|
||||
independent reports and return aggregate failure through CLI.
|
||||
- Tests to inspect: `internal/app/app_test.go`, CLI batch tests, state tests.
|
||||
- Architectural invariants: render preflight precedes run; domain logic stays
|
||||
outside adapters; metadata links artifacts.
|
||||
|
||||
- Weather data
|
||||
- Path: `docs/internal/weather-data.md`
|
||||
- Purpose: document Weather API fan-out into `forecast.Bundle`.
|
||||
- Inputs and outputs: config, HTTP responses, source records, warnings,
|
||||
bundle.
|
||||
- Boundaries: adapter handles transport and normalization boundary, not
|
||||
forecast selection policy.
|
||||
- Config fields used: base URL, timeout, precision, units, timezone, format,
|
||||
missing-source policy.
|
||||
- Adapters used: Weather API HTTP client.
|
||||
- Failure behavior: hourly is required; optional and stub sources follow
|
||||
missing-source policy.
|
||||
- Tests to inspect: `internal/adapters/weatherapi/client_test.go` and
|
||||
adapter testdata.
|
||||
- Architectural invariants: source provenance and warning records are
|
||||
first-class.
|
||||
|
||||
- Forecast derivation
|
||||
- Path: `docs/internal/forecast-derivation.md`
|
||||
- Purpose: document deriving daily and period summaries from bundle data.
|
||||
- Inputs and outputs: bundle, dayparts, valid period, summary structures,
|
||||
warnings.
|
||||
- Boundaries: no HTTP, state, or Scriptorium calls.
|
||||
- Config fields used: dayparts and relevant threshold settings.
|
||||
- Adapters used: none directly.
|
||||
- Failure behavior: missing required hourly data fails report preparation.
|
||||
- Tests to inspect: `internal/forecast/derive_test.go`.
|
||||
- Architectural invariants: Go owns period selection and meteorological
|
||||
summarization.
|
||||
|
||||
- Report registry
|
||||
- Path: `docs/internal/report-registry.md`
|
||||
- Purpose: document report IDs, prompt IDs, valid periods, output naming,
|
||||
batches, and comparison strategies.
|
||||
- Inputs and outputs: report kind, clock, timezone, manual bounds, resolved
|
||||
report definition.
|
||||
- Boundaries: does not build briefings or execute workflows.
|
||||
- Config fields used: timezone and report output settings.
|
||||
- Adapters used: none.
|
||||
- Failure behavior: invalid report kinds, invalid dates, and invalid manual
|
||||
storm bounds fail before generation.
|
||||
- Tests to inspect: `internal/report/period_test.go`.
|
||||
- Architectural invariants: report-specific behavior is registry-driven.
|
||||
|
||||
- Briefing
|
||||
- Path: `docs/internal/briefing.md`
|
||||
- Purpose: document report-specific briefing package builders.
|
||||
- Inputs and outputs: resolved report, forecast summaries, source metadata,
|
||||
source warnings, briefing package.
|
||||
- Boundaries: no prompt rendering and no external calls.
|
||||
- Config fields used: dayparts and report period inputs.
|
||||
- Adapters used: none directly.
|
||||
- Failure behavior: invalid or insufficient forecast summaries fail before
|
||||
prompt input construction.
|
||||
- Tests to inspect: briefing package tests.
|
||||
- Architectural invariants: briefings are curated inputs for prompts.
|
||||
|
||||
- Prompt input
|
||||
- Path: `docs/internal/prompt-input.md`
|
||||
- Purpose: document data package construction for Scriptorium.
|
||||
- Inputs and outputs: metadata, briefing, recent changes, warnings,
|
||||
`data_package` JSON.
|
||||
- Boundaries: no subprocess execution.
|
||||
- Config fields used: none directly except values already recorded in
|
||||
metadata/briefing.
|
||||
- Adapters used: none directly.
|
||||
- Failure behavior: validation errors fail before render/run.
|
||||
- Tests to inspect: `internal/promptinput/package_test.go`.
|
||||
- Architectural invariants: Scriptorium receives structured prompt input, not
|
||||
raw unbounded source payloads.
|
||||
|
||||
- Recent changes
|
||||
- Path: `docs/internal/changes.md`
|
||||
- Purpose: document structured comparison of current and prior briefings.
|
||||
- Inputs and outputs: current briefing, prior comparable briefing, thresholds,
|
||||
change items.
|
||||
- Boundaries: no Markdown comparison and no external calls.
|
||||
- Config fields used: `recent_change` thresholds.
|
||||
- Adapters used: none.
|
||||
- Failure behavior: no prior comparable snapshot produces an empty change set.
|
||||
- Tests to inspect: `internal/changes/*_test.go`.
|
||||
- Architectural invariants: comparisons are structured and report-compatible.
|
||||
|
||||
- Filesystem state
|
||||
- Path: `docs/internal/state.md`
|
||||
- Purpose: document workspace paths, metadata, artifact persistence, lookup,
|
||||
and inspection support.
|
||||
- Inputs and outputs: workspace config, RunID, artifact JSON/Markdown,
|
||||
metadata records, lookup results.
|
||||
- Boundaries: no forecast logic and no subprocess execution.
|
||||
- Config fields used: workspace directories and report output directory.
|
||||
- Adapters used: filesystem.
|
||||
- Failure behavior: path, write, read, and lookup errors are surfaced with
|
||||
context.
|
||||
- Tests to inspect: `internal/state/filesystem_test.go`.
|
||||
- Architectural invariants: persisted artifacts support inspection and retry
|
||||
diagnosis.
|
||||
|
||||
- Scriptorium adapter
|
||||
- Path: `docs/internal/scriptorium-adapter.md`
|
||||
- Purpose: document subprocess isolation and result handling.
|
||||
- Inputs and outputs: render/run requests, argv, stdout, stderr, exit code,
|
||||
report path.
|
||||
- Boundaries: owns subprocess invocation only; does not know report semantics.
|
||||
- Config fields used: binary, config path, profile, timeout, extra args.
|
||||
- Adapters used: external `scriptorium` CLI.
|
||||
- Failure behavior: context timeouts and nonzero exits return captured output.
|
||||
- Tests to inspect: `internal/adapters/scriptorium/runner_test.go`.
|
||||
- Architectural invariants: no shell interpolation; Scriptorium details do not
|
||||
leak into domain packages.
|
||||
|
||||
## Integration Documentation Plan
|
||||
|
||||
- Weather API
|
||||
- Path: `docs/integrations/weatherapi.md`
|
||||
- External system or contract: internal weatherfeeder-backed Weather API.
|
||||
- Current usage: the adapter fetches observations, current conditions, hourly
|
||||
forecast, narrative forecast, active alerts, and discussion; daily forecast
|
||||
and weather story are internal stub source slots.
|
||||
- Version or compatibility notes: no explicit external version is visible in
|
||||
the repository; document compatibility in terms of endpoints, query
|
||||
parameters, and response envelope shape used by tests.
|
||||
- What should be documented: base URL joining, timeout behavior, query
|
||||
parameters (`format`, `units`, `tz`, `precision` where used), `data`
|
||||
envelope handling, `data:null`, source hashes, warnings, and required vs
|
||||
optional sources.
|
||||
- What should not be documented: unused endpoint families, unimplemented
|
||||
location selection, upstream implementation internals, or private
|
||||
deployment details.
|
||||
|
||||
- Scriptorium
|
||||
- Path: `docs/integrations/scriptorium.md`
|
||||
- External system or contract: `scriptorium` CLI.
|
||||
- Current usage: `scriptorium render --prompt <prompt_id> --input
|
||||
data_package=<path> --format json` and `scriptorium run --prompt
|
||||
<prompt_id> --input data_package=<path> --out <artifact_path>`, with
|
||||
configured binary/config/profile/extra args where applicable.
|
||||
- Version or compatibility notes: no explicit Scriptorium version is visible
|
||||
in the repository; document the argv contract and behavior expected by
|
||||
adapter tests.
|
||||
- What should be documented: input contract, output path behavior,
|
||||
stdout/stderr capture, nonzero exit handling, timeout behavior, and security
|
||||
notes about no shell interpolation.
|
||||
- What should not be documented: unused Scriptorium modes, prompt authoring
|
||||
guidance beyond the input contract, or Scriptorium internals.
|
||||
|
||||
## Recommended Implementation Sequence
|
||||
|
||||
### Stage 1: Baseline And User Docs
|
||||
|
||||
- Goal: align README, CLI, and config docs with implemented behavior.
|
||||
- Files to create/update/delete/move: update `README.md`, `docs/cli.md`,
|
||||
`docs/config.md`.
|
||||
- Repository areas to inspect: `cmd/weatherreporter/main.go`,
|
||||
`internal/cli/root.go`, `internal/cli/root_test.go`, `internal/config/*`,
|
||||
`internal/config/config_test.go`, `examples/config.yml`.
|
||||
- Acceptance criteria: no unimplemented commands or config fields; quickstart
|
||||
commands match `weatherreporter --help`; config defaults and precedence match
|
||||
code; README remains concise.
|
||||
- Suggested validation commands: `go run ./cmd/weatherreporter --help`,
|
||||
`go test ./internal/cli ./internal/config`, stale-term greps for removed CLI
|
||||
or prompt terminology.
|
||||
- Prompt size: small enough for one implementation prompt.
|
||||
|
||||
### Stage 2: Operations And Troubleshooting
|
||||
|
||||
- Goal: keep operations focused on normal workflows and recovery; create a
|
||||
symptom-driven troubleshooting guide.
|
||||
- Files to create/update/delete/move: update `docs/operations.md`; create
|
||||
`docs/troubleshooting.md`.
|
||||
- Repository areas to inspect: `internal/app/app.go`, `internal/app/inspect.go`,
|
||||
`internal/state/*`, `internal/report/*`, `internal/adapters/weatherapi/*`,
|
||||
`internal/adapters/scriptorium/*`.
|
||||
- Acceptance criteria: recovery and troubleshooting are clearly separated;
|
||||
failure entries are actionable; state layout matches code; no resume,
|
||||
cleanup, archive, remote storage, or daemon behavior is documented as
|
||||
implemented.
|
||||
- Suggested validation commands: `go test ./internal/app ./internal/state`,
|
||||
`go run ./cmd/weatherreporter --help`, link review.
|
||||
- Prompt size: small enough for one implementation prompt.
|
||||
|
||||
### Stage 3: Internal Component Docs
|
||||
|
||||
- Goal: bring internal docs into the policy-required shape and add missing app
|
||||
orchestration documentation.
|
||||
- Files to create/update/delete/move: create
|
||||
`docs/internal/app-orchestration.md`; update all existing `docs/internal/*.md`.
|
||||
- Repository areas to inspect: `internal/app`, `internal/forecast`,
|
||||
`internal/briefing`, `internal/promptinput`, `internal/changes`,
|
||||
`internal/report`, `internal/state`, adapters, and related tests.
|
||||
- Acceptance criteria: each component doc includes purpose, inputs/outputs,
|
||||
boundaries, config fields, adapters, state behavior, skip/resume behavior,
|
||||
failure behavior, tests, and invariants; no unimplemented components are
|
||||
documented.
|
||||
- Suggested validation commands: `go test ./internal/...`, grep for stale terms
|
||||
in `docs/internal`.
|
||||
- Prompt size: likely too large for one prompt unless handled mechanically;
|
||||
split into app/state/adapters and domain/report/briefing docs if needed.
|
||||
|
||||
### Stage 4: Integration Contracts
|
||||
|
||||
- Goal: make integration docs concise and limited to actual external contracts
|
||||
used by `weatherreporter`.
|
||||
- Files to create/update/delete/move: rewrite
|
||||
`docs/integrations/weatherapi.md`; lightly update
|
||||
`docs/integrations/scriptorium.md`.
|
||||
- Repository areas to inspect: `internal/adapters/weatherapi/*`,
|
||||
weather adapter testdata, `internal/adapters/scriptorium/*`,
|
||||
`internal/config/*`.
|
||||
- Acceptance criteria: no unused Weather API endpoint surface is presented as
|
||||
current behavior; Scriptorium docs use `--input data_package=<path>`; no
|
||||
`--vars-file` or `promptvars` terminology.
|
||||
- Suggested validation commands: `go test ./internal/adapters/weatherapi
|
||||
./internal/adapters/scriptorium`, stale-term greps.
|
||||
- Prompt size: small enough for one implementation prompt.
|
||||
|
||||
### Stage 5: Policy Docs
|
||||
|
||||
- Goal: align policy docs with their canonical roles.
|
||||
- Files to create/update/delete/move: rewrite
|
||||
`docs/policy/development.md`; lightly update
|
||||
`docs/policy/architecture.md` only if implementation inspection finds
|
||||
inaccuracies; leave `docs/policy/documentation.md` unchanged unless the
|
||||
project intentionally changes policy.
|
||||
- Repository areas to inspect: `go.mod`, package layout, `internal/cli`,
|
||||
`internal/config`, adapters, tests, current documentation policy.
|
||||
- Acceptance criteria: development policy is contributor workflow, not proposed
|
||||
architecture; future/planned/MVP language is removed or moved to roadmap;
|
||||
architecture policy remains principle-level.
|
||||
- Suggested validation commands: grep `docs/policy` for feature-specific
|
||||
future/planned/MVP wording, `go test ./...`.
|
||||
- Prompt size: small enough for one implementation prompt.
|
||||
|
||||
### Stage 6: Examples And Roadmap Cleanup
|
||||
|
||||
- Goal: validate examples and ensure roadmap docs contain only future/planned
|
||||
material.
|
||||
- Files to create/update/delete/move: update `examples/config.yml`; optionally
|
||||
create `examples/minimal-config.yml` with validation coverage; keep
|
||||
`docs/roadmap/future.md` as the future-only roadmap after removing stale
|
||||
implementation-history material.
|
||||
- Repository areas to inspect: `internal/config/*`, config tests,
|
||||
roadmap docs, current implemented feature set.
|
||||
- Acceptance criteria: examples load successfully; roadmap files are clearly
|
||||
future-only; no completed MVP stage plan is linked as current docs.
|
||||
- Suggested validation commands: config example loading test, `go test ./...`,
|
||||
link review.
|
||||
- Prompt size: small enough for one implementation prompt if no new example
|
||||
test helper is needed; otherwise split examples and roadmap cleanup.
|
||||
|
||||
### Stage 7: Final Validation
|
||||
|
||||
- Goal: verify the documentation set is coherent and policy-compliant.
|
||||
- Files to create/update/delete/move: all migrated docs and examples.
|
||||
- Repository areas to inspect: full repository.
|
||||
- Acceptance criteria: non-roadmap docs describe only implemented behavior;
|
||||
canonical homes are respected; links are valid; examples are maintained;
|
||||
stale terminology is absent.
|
||||
- Suggested validation commands: `go test ./...`,
|
||||
`go run ./cmd/weatherreporter --help`, `git diff --check`, stale-term greps,
|
||||
manual link review.
|
||||
- Prompt size: small enough for one implementation prompt.
|
||||
|
||||
## Validation Plan
|
||||
|
||||
Automated or semi-automated checks:
|
||||
|
||||
- Run `go test ./...` after documentation migration, especially if examples or
|
||||
config-loading tests change.
|
||||
- Run `go run ./cmd/weatherreporter --help` and compare documented CLI syntax
|
||||
against the output.
|
||||
- Run `git diff --check` for whitespace and patch hygiene.
|
||||
- Search for stale terminology outside roadmap docs:
|
||||
- `--vars-file`
|
||||
- `promptvars`
|
||||
- `weather.tomorrow_report`
|
||||
- `--location home`
|
||||
- `MVP`
|
||||
- `future`
|
||||
- `planned`
|
||||
- `proposed`
|
||||
- Review matches manually. Some policy-level wording such as documentation
|
||||
policy references to future work is legitimate; feature-specific future
|
||||
behavior outside `docs/roadmap/` is not.
|
||||
- Verify README, CLI, config, operations, troubleshooting, internal, and
|
||||
integration links manually. No automated link checker is currently present.
|
||||
- If `examples/minimal-config.yml` is added, add or update a config-loading test
|
||||
so the example remains maintained.
|
||||
- Verify `docs/integrations/weatherapi.md` against
|
||||
`internal/adapters/weatherapi/client.go` so unused endpoints are not
|
||||
documented as current usage.
|
||||
- Verify `docs/integrations/scriptorium.md` against
|
||||
`internal/adapters/scriptorium/runner.go` so argv examples match current
|
||||
subprocess construction.
|
||||
|
||||
Manual review items:
|
||||
|
||||
- Confirm each non-roadmap document has a clear audience and canonical scope.
|
||||
- Confirm current-behavior docs avoid changelog or development-history framing.
|
||||
- Confirm roadmap docs clearly distinguish accepted plans, proposed work,
|
||||
deferred ideas, and rejected ideas where applicable.
|
||||
- Confirm internal docs describe implemented components only.
|
||||
- Confirm operations and troubleshooting do not imply resume, cleanup, archive,
|
||||
remote storage, daemon, or automatic storm monitoring support.
|
||||
|
||||
## Open Questions
|
||||
|
||||
No questions block a correct documentation roadmap or migration.
|
||||
|
||||
Recommendation: keep `docs/roadmap/future.md` future-only. Do not reintroduce
|
||||
completed implementation-history material as current project documentation.
|
||||
Reference in New Issue
Block a user