Document CLI output contract
This commit is contained in:
@@ -1,326 +0,0 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user