From f4f009b9043751fab24572ec733d2e219e0cd410 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 20 Jun 2026 17:39:05 -0500 Subject: [PATCH] Add a feature roadmap and staged implentation plan to harmonize CLI command outputs --- docs/roadmap/cli.md | 241 ++++++++++++++++++++++++ docs/roadmap/implementation.md | 326 +++++++++++++++++++++++++++++++++ 2 files changed, 567 insertions(+) create mode 100644 docs/roadmap/cli.md create mode 100644 docs/roadmap/implementation.md diff --git a/docs/roadmap/cli.md b/docs/roadmap/cli.md new file mode 100644 index 0000000..6f59c5d --- /dev/null +++ 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. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..9ff59f5 --- /dev/null +++ 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.