Add a feature roadmap and staged implentation plan to harmonize CLI command outputs

2026-06-20 17:39:05 -05:00
parent 3c1b753952
commit f4f009b904
2 changed files with 567 additions and 0 deletions
--- a/docs/roadmap/cli.md
+++ b/docs/roadmap/cli.md
@@ -0,0 +1,241 @@
+# CLI Output Roadmap
+
+## Purpose
+
+This roadmap defines the intended final shape for weatherreporter CLI output.
+
+The current CLI has drifted:
+
+- `run` commands emit JSON summaries to stdout and compact status lines to
+  stderr.
+- `inspect` commands emit JSON to stdout.
+- `generate` commands perform substantial work but are silent on success.
+
+The target is a predictable command-line contract that is useful for operators,
+easy to consume from scripts, and explicit enough that future commands naturally
+reuse the same output path.
+
+## Locked Decisions
+
+- Keep application orchestration and domain decisions in `internal/app`.
+- Keep CLI presentation, stdout/stderr policy, and quiet-mode behavior in
+  `internal/cli`.
+- Successful non-help commands should have a machine-readable JSON stdout
+  contract unless `--quiet` intentionally suppresses success output for an
+  action command.
+- Help remains human-readable text.
+- Stderr is for compact operational status and errors, not primary command
+  payloads.
+- Do not print partial JSON when command construction, flag parsing, config
+  loading, or pre-run validation fails.
+- Do not serialize large internal app objects directly as CLI output.
+- Do not expose secret values in stdout or stderr.
+- Add `--quiet` for state-changing action commands.
+- Do not make `--quiet` suppress requested inspection data.
+
+## Command Categories
+
+CLI commands should be classified into one of these output categories.
+
+### Help Commands
+
+Examples:
+
+- `weatherreporter --help`
+
+Output:
+
+- stdout: human-readable help text
+- stderr: none on success
+- `--quiet`: not applicable
+
+### Action Commands
+
+Examples:
+
+- `weatherreporter generate today`
+- `weatherreporter generate daily --date YYYY-MM-DD`
+- `weatherreporter run morning`
+- `weatherreporter run evening`
+
+Output:
+
+- stdout: compact JSON summary after the action completes
+- stderr: compact status lines only when useful, especially for multi-report
+  batch commands
+- `--quiet`: suppress success stdout and routine status stderr
+
+Failure behavior:
+
+- For flag/config/pre-run errors, stdout is empty and the command returns an
+  error.
+- For completed actions that produce an inspectable failure result, default
+  output may still include a JSON failure summary before returning nonzero.
+- With `--quiet`, failure diagnostics should remain concise and actionable on
+  stderr through the existing top-level error path; routine success summaries
+  stay suppressed.
+
+### Inspection Commands
+
+Examples:
+
+- `weatherreporter inspect reports`
+- `weatherreporter inspect metadata RUN_ID`
+- `weatherreporter inspect modules RUN_ID`
+- `weatherreporter inspect data-package RUN_ID`
+- `weatherreporter inspect prior RUN_ID`
+- `weatherreporter inspect sources RUN_ID`
+
+Output:
+
+- stdout: requested JSON data
+- stderr: none on success
+- `--quiet`: not accepted unless a future inspection command has auxiliary
+  status output to suppress
+
+Inspection commands are already data-oriented. Their stdout payload should stay
+focused on the requested data rather than being hidden by quiet mode.
+
+## Target Action Summary Shape
+
+Action command JSON should be small, stable, and path-oriented. It should expose
+what an operator needs to find artifacts, inspect a run, and understand
+notification status.
+
+### Generate Summary
+
+Target shape:
+
+```json
+{
+  "command": "generate",
+  "reportId": "today",
+  "reportName": "Today Report",
+  "promptId": "weather.today_generated_text",
+  "runId": "20260529T100000.000000000Z_today",
+  "status": "succeeded",
+  "generatedAt": "2026-05-29T10:00:00Z",
+  "validPeriod": {},
+  "reportPath": "workspace/reports/today/2026-05-29/report.20260529T100000.000000000Z_today.md",
+  "outputPath": "./today.md",
+  "metadataPath": "workspace/snapshots/today/2026-05-29/metadata.20260529T100000.000000000Z_today.json",
+  "dataPackagePath": "workspace/data-packages/today/2026-05-29/data_package.20260529T100000.000000000Z_today.yaml",
+  "preflightPath": "workspace/preflight/today/2026-05-29/render.20260529T100000.000000000Z_today.json",
+  "notificationPath": "workspace/notifications/today/2026-05-29/distributor.20260529T100000.000000000Z_today.json",
+  "notification": {
+    "status": "succeeded",
+    "runId": "distributor-run",
+    "pipelineId": "weatherreporter.today",
+    "bundleId": "weatherreporter.home.today",
+    "path": "workspace/notifications/today/2026-05-29/distributor.20260529T100000.000000000Z_today.json"
+  }
+}
+```
+
+Rules:
+
+- Omit absent optional paths with `omitempty`.
+- Include generated-text artifact paths only for report modes that produce them.
+- Include notification fields only when notification was attempted.
+- Keep module snapshot contents, data package contents, raw generated text,
+  render result bodies, and full notification adapter payloads out of the CLI
+  summary.
+
+### Batch Summary
+
+The current `BatchResult` shape is close to the target and should remain the
+basis for `run` output. The target update is to make the summary explicitly
+command-like and align status semantics with generate output:
+
+```json
+{
+  "command": "run",
+  "batch": "morning",
+  "status": "succeeded",
+  "startedAt": "2026-05-29T10:00:00Z",
+  "finishedAt": "2026-05-29T10:01:00Z",
+  "total": 3,
+  "succeeded": 3,
+  "failed": 0,
+  "notification": {},
+  "reports": []
+}
+```
+
+Rules:
+
+- Keep per-report items compact and path-oriented.
+- Keep batch notification status at the top level.
+- Preserve nonzero exit behavior when one or more reports fail.
+- Preserve the existing behavior that batch report failures do not prevent the
+  JSON summary from being available in default output mode.
+
+## Quiet Mode
+
+`--quiet` should be available on action commands:
+
+```sh
+weatherreporter generate today --quiet
+weatherreporter run morning --quiet
+```
+
+Quiet mode means:
+
+- no stdout on successful action commands
+- no routine status lines on stderr on successful action commands
+- errors still return nonzero and are still reported by the top-level CLI error
+  path
+- inspection output is not suppressed
+
+Quiet mode does not mean:
+
+- skipping artifact writes
+- skipping distributor notification
+- changing JSON shape when JSON is emitted
+- hiding errors
+
+Future action commands should opt into quiet mode by using the centralized
+action-output writer rather than implementing their own flag or writer logic.
+
+## Intended Code Structure
+
+`internal/cli` should own a small output layer that future commands can reuse.
+The output layer should make the consistent path the easiest path.
+
+Target files:
+
+- `internal/cli/root.go`: command routing and flag parsing
+- `internal/cli/output.go`: stdout/stderr writers, quiet-mode handling, and
+  output category helpers
+- `internal/cli/result.go`: CLI-safe summary structs and conversion helpers
+
+Target app-layer shape:
+
+- Add a detailed generate entry point that returns the generated report result.
+- Keep `app.Generate(ctx, GenerateRequest) error` as a convenience wrapper for
+  callers that do not need CLI output.
+- Keep `app.RunBatchDetailed(ctx, BatchRequest) (*BatchResult, error)` as the
+  batch command result source.
+
+Target CLI output helpers:
+
+- `writeJSON(io.Writer, any) error`
+- `writeActionResult(stdout, stderr io.Writer, result actionResult, opts outputOptions) error`
+- `writeBatchStatus(stderr io.Writer, result *app.BatchResult)`
+- `writeGenerateStatus(stderr io.Writer, result GenerateSummary)` only if
+  single-report status lines become useful
+
+The command router should not call `json.NewEncoder` directly outside the
+central output helpers.
+
+## Deferred Questions
+
+None of these are required for the initial harmonization:
+
+- global `--format` support
+- NDJSON progress streams
+- human-readable success output
+- machine-readable error envelopes on stderr
+- making inspection commands use a common envelope
+
+These should remain deferred until there is a real consumer need.
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -0,0 +1,326 @@
+# CLI Output Implementation Plan
+
+## Purpose
+
+This document is the staged implementation plan for
+[cli.md](cli.md). It is written for an LLM coding agent that will implement the
+CLI output harmonization in order.
+
+The feature is complete when `generate`, `run`, and `inspect` have consistent
+stdout/stderr behavior, action commands support `--quiet`, CLI output logic is
+centralized in `internal/cli`, and maintained docs describe the implemented
+contract.
+
+## Ground Rules
+
+- Review `docs/policy/architecture.md`, `docs/policy/development.md`, and
+  `docs/policy/documentation.md` before editing code.
+- Keep application orchestration and domain behavior in `internal/app`.
+- Keep CLI presentation, output summaries, stdout/stderr policy, and quiet-mode
+  behavior in `internal/cli`.
+- Do not serialize full `app.ReportResult` values directly to CLI stdout.
+- Do not add `--quiet` to inspection commands in this pass.
+- Do not add global `--format`, NDJSON progress, human-readable success output,
+  or machine-readable error envelopes.
+- Preserve current command semantics except where this plan explicitly changes
+  output behavior.
+
+## Decisions
+
+- CLI-safe summary structs live in `internal/cli/result.go`, not
+  `internal/app`.
+- Add `app.GenerateDetailed(ctx, GenerateRequest) (*ReportResult, error)`.
+- Keep `app.Generate(ctx, GenerateRequest) error` as a wrapper around
+  `GenerateDetailed`.
+- Keep `app.RunBatchDetailed(ctx, BatchRequest) (*BatchResult, error)` as the
+  app-layer batch source.
+- Add `command` and `status` fields in CLI summary structs, not in
+  `app.BatchResult`.
+- `BatchSummary.status` is `failed` when any report failed or the top-level
+  batch notification status is `failed`; otherwise it is `succeeded`.
+- `GenerateSummary.status` is `succeeded` for a successful generated report and
+  `failed` only when a non-nil `ReportResult` is returned with an error after
+  inspectable artifacts exist.
+- Default action-command output may include a failure JSON summary when the app
+  layer returns a non-nil result with an error.
+- Quiet mode suppresses successful action-command stdout and routine stderr. It
+  does not hide returned errors.
+
+## Stage 1: Detailed Generate Result
+
+Goal: make single-report generation return the same kind of structured result
+that batch generation already uses internally.
+
+Implementation:
+
+- Add `GenerateDetailed(ctx, GenerateRequest) (*ReportResult, error)` in
+  `internal/app`.
+- Move the current body of `Generate` into `GenerateDetailed`.
+- Change `Generate` to call `GenerateDetailed` and return only the error.
+- Ensure `GenerateDetailed` preserves existing behavior for:
+  - collecting weather before resolving/generating;
+  - unknown or unimplemented reports;
+  - explicit Daily date requirements;
+  - optional output copy behavior;
+  - distributor notification behavior.
+- When `GenerateReport` or generated-template finalization receives a non-empty
+  `finalizeRenderedReportResult` plus an error after managed artifacts exist,
+  return a non-nil `ReportResult` together with that error. This is especially
+  important for notification failures where the report and notification artifact
+  are inspectable.
+- Do not return partial results for flag/config/pre-run validation failures or
+  failures before a useful run identity exists.
+
+Tests:
+
+- Add app tests for `GenerateDetailed` success.
+- Add an app test showing `Generate` still returns only the underlying error.
+- Add or adjust an app test for notification failure so `GenerateDetailed`
+  returns a non-nil result with report, metadata, and notification artifact
+  paths while also returning the notification error.
+
+Validation:
+
+```bash
+go test ./internal/app
+```
+
+Completion criteria:
+
+- Existing app behavior is preserved for callers of `Generate`.
+- CLI callers can obtain a rich `ReportResult` from `GenerateDetailed`.
+
+## Stage 2: CLI Summary Types
+
+Goal: define small, stable CLI output contracts without exposing full app
+internals.
+
+Implementation:
+
+- Add `internal/cli/result.go`.
+- Define a `generateSummary` struct with these JSON fields:
+  - `command`
+  - `reportId`
+  - `reportName`
+  - `promptId`
+  - `runId`
+  - `status`
+  - `generatedAt`
+  - `validPeriod`
+  - `reportPath,omitempty`
+  - `outputPath,omitempty`
+  - `metadataPath,omitempty`
+  - `dataPackagePath,omitempty`
+  - `preflightPath,omitempty`
+  - `generatedTextRawPath,omitempty`
+  - `generatedTextResultPath,omitempty`
+  - `generatedTextPath,omitempty`
+  - `renderContextPath,omitempty`
+  - `notificationPath,omitempty`
+  - `notification,omitempty`
+  - `error,omitempty`
+- Define a `batchSummary` struct with these JSON fields:
+  - `command`
+  - `batch`
+  - `status`
+  - `startedAt`
+  - `finishedAt`
+  - `total`
+  - `succeeded`
+  - `failed`
+  - `notification,omitempty`
+  - `reports`
+  - `error,omitempty`
+- Reuse existing app result substructures where they are already CLI-safe:
+  `timeutil.Period`, `app.BatchNotificationResult`, and
+  `app.BatchReportResult`.
+- Add conversion helpers:
+  - `newGenerateSummary(result *app.ReportResult, err error) generateSummary`
+  - `newBatchSummary(result *app.BatchResult) batchSummary`
+- Do not include module snapshot contents, data package contents, raw generated
+  text bytes, render result bodies, or full distributor adapter payloads.
+- Keep error strings concise and avoid adding secrets.
+
+Tests:
+
+- Add focused unit tests for summary conversion.
+- Cover generated-text reports, markdown reports, disabled notification, and
+  notification failure with a non-nil result.
+- Cover batch status derivation for success, report failure, skipped
+  notification, and failed notification.
+
+Validation:
+
+```bash
+go test ./internal/cli
+```
+
+Completion criteria:
+
+- CLI summary shapes are explicit and independent of full app result structs.
+
+## Stage 3: Centralized Output Helpers
+
+Goal: make the consistent output path the default path for current and future
+commands.
+
+Implementation:
+
+- Add `internal/cli/output.go`.
+- Move `writeJSON` from `root.go` into `output.go`.
+- Move `writeRunLogs` from `root.go` into `output.go` and rename it to
+  `writeBatchStatus`.
+- Add an `outputOptions` struct with at least:
+  - `Quiet bool`
+- Add a small action output helper such as:
+  `writeActionResult(stdout, stderr io.Writer, value any, opts outputOptions, writeStatus func(io.Writer)) error`.
+- The helper must:
+  - return without writing stdout or routine stderr when `opts.Quiet` is true;
+  - write status before JSON for default action output when a status writer is
+    provided;
+  - use the shared JSON writer for stdout;
+  - tolerate nil stderr when no status output is needed.
+- Keep inspect commands using `writeJSON` directly because inspection is data
+  output, not quietable action output.
+- Ensure `root.go` no longer calls `json.NewEncoder` directly.
+
+Tests:
+
+- Add unit tests for quiet/default action output helper behavior.
+- Keep existing batch stderr tests, updated for renamed helpers if needed.
+
+Validation:
+
+```bash
+go test ./internal/cli
+```
+
+Completion criteria:
+
+- JSON encoding and action status output are centralized outside command
+  routing.
+
+## Stage 4: Wire Generate And Run Output
+
+Goal: make current action commands use the same output contract.
+
+Implementation:
+
+- Extend `commonOptions` or action-specific options with `Quiet bool`.
+- Parse `--quiet` for `generate` and `run`.
+- Do not parse or accept `--quiet` for `inspect`.
+- Update `Runner.Run`:
+  - `generate` should call `app.GenerateDetailed`;
+  - when a non-nil result is returned, convert it to `generateSummary`;
+  - write the summary through the centralized action output helper;
+  - if an error is also returned, write default JSON only when a non-nil result
+    exists and quiet is false, then return the error;
+  - if no result is returned, return the error without writing partial JSON.
+- Update `run` command handling:
+  - convert `*app.BatchResult` to `batchSummary`;
+  - write through the centralized action output helper;
+  - preserve the current behavior that a batch result is emitted in default
+    mode before returning `app.BatchError` for failed reports;
+  - preserve notification-only batch failure behavior.
+- Keep `inspect` commands unchanged except for using the relocated `writeJSON`.
+
+Tests:
+
+- `generate today` emits valid JSON on success.
+- The generate JSON includes `command: "generate"`, `status: "succeeded"`,
+  `reportId`, `runId`, `reportPath`, `metadataPath`, `dataPackagePath`, and
+  `preflightPath`.
+- Generated-text reports include generated-text artifact paths.
+- Markdown reports omit generated-text artifact paths.
+- `generate --quiet` emits no stdout or routine stderr on success.
+- Generate pre-run errors emit no partial JSON.
+- Generate notification failure with an inspectable result emits a failure JSON
+  summary in default mode and returns nonzero.
+- `run morning` and `run evening` still emit JSON summaries by default.
+- Run JSON includes `command: "run"` and a derived `status`.
+- `run --quiet` suppresses successful summary/status output.
+- Failed batch runs still return nonzero and still emit default JSON when quiet
+  is false.
+- Inspect commands still emit requested JSON and reject `--quiet` as an
+  unexpected flag.
+- Output tests confirm distributor token values are not printed.
+
+Validation:
+
+```bash
+go test ./internal/cli ./internal/app
+```
+
+Completion criteria:
+
+- Current action commands have consistent default JSON behavior.
+- Quiet mode is available for action commands and not inspection commands.
+
+## Stage 5: Documentation
+
+Goal: document the implemented CLI output contract in maintained docs and make
+future changes follow the same structure.
+
+Documentation changes:
+
+- Update `docs/cli.md`:
+  - document default JSON stdout for `generate`, `run`, and `inspect`;
+  - document compact status stderr for batch commands;
+  - document `--quiet` for `generate` and `run`;
+  - include representative generate and run JSON snippets;
+  - state that inspection commands are not quietable.
+- Update `docs/operations.md` if cron/operator behavior changes need an
+  operations note.
+- Add `docs/internal/cli.md` documenting:
+  - command categories;
+  - stdout/stderr rules;
+  - quiet-mode behavior;
+  - summary conversion ownership;
+  - the expected helper path for future commands.
+- Update `docs/policy/development.md` CLI-change guidance so future CLI
+  commands are expected to use the centralized output helpers and declare an
+  output category.
+
+Tests:
+
+- Update CLI help-output tests for `--quiet`.
+- Add or update docs-related tests only if this repository already validates the
+  touched docs/examples in tests.
+
+Validation:
+
+```bash
+go test ./internal/cli ./internal/app
+go test ./...
+go run ./cmd/weatherreporter --help
+git diff --check
+```
+
+Completion criteria:
+
+- Non-roadmap docs describe only implemented behavior.
+- The roadmap can be removed after implementation if no deferred CLI-output
+  feature remains in it.
+
+## Final Verification
+
+Before considering the feature complete, run:
+
+```bash
+go test ./...
+go run ./cmd/weatherreporter --help
+git diff --check
+```
+
+Also manually verify these command behaviors against test fixtures or a local
+test config when practical:
+
+```bash
+weatherreporter generate today --quiet
+weatherreporter run morning --quiet
+weatherreporter inspect reports --limit 1
+```
+
+## Open Questions
+
+None. The decisions above are sufficient for implementation.