327 lines
11 KiB
Markdown
327 lines
11 KiB
Markdown
# 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.
|