From 3b5a6d26d6af42bb15f32c9e2a8958353c0c027c Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Sat, 4 Jul 2026 00:45:26 +0800 Subject: [PATCH] docs: design for concise output + diff-scoped any check Co-Authored-By: Claude Opus 4.8 --- ...4-concise-output-diff-scoped-any-design.md | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-04-concise-output-diff-scoped-any-design.md diff --git a/docs/superpowers/specs/2026-07-04-concise-output-diff-scoped-any-design.md b/docs/superpowers/specs/2026-07-04-concise-output-diff-scoped-any-design.md new file mode 100644 index 0000000..da1b1cf --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-concise-output-diff-scoped-any-design.md @@ -0,0 +1,147 @@ +# Concise output + diff-scoped `any` — design + +Date: 2026-07-04 + +## Problem + +`check-safety` prints a `=== Check N: Title ===` header plus an `OK`/`SKIP`/`FAIL` +line for every one of ~30 checks, every run. A clean run is ~120 lines. When the +tool's output is fed back into an agent's context (the common case here), that is +pure token waste — the interesting content is only the failures. + +Second issue: the `any`-usage check (2e) scans **every** source file every run, so +it surfaces the entire pre-existing backlog of `any` on every invocation. In normal +use we only care about `any` we are about to introduce. + +## Goals + +1. Default output communicates only what matters — failures/warnings with + `file:line`, plus a one-line summary — using far fewer tokens. Full detail stays + available behind `--verbose`. +2. The `any` check defaults to only the **unstaged working-tree diff** (changed + lines), with `--all-any` to restore the full scan. +3. No change to the CLI contract (`check-safety [--flags]`) or exit-code + semantics. + +## Design + +### 1. Reporter becomes a collector + +Root cause of the verbosity is that each `check_*.go` prints its own header and +result lines inline via `fmt.Print*`. That makes central formatting impossible. + +`registry.go`'s `Reporter` is rewritten from a bare exit-code holder into a +per-check collector. The `main` loop owns each check's lifecycle: + +```go +for i := range registry { + c := registry[i] + rep.begin(c.Seq, c.ID, c.Title) // sets rep.current, default status OK + c.Run(ctx, rep) + rep.end() // appends current to rep.reports +} +rep.render() +os.Exit(rep.exitCode) +``` + +Checks stop printing and instead declare status + findings: + +```go +rep.OK("clean") // status OK, optional one-line summary +rep.Skip("no frontend sources") // status SKIP +rep.Warn("...") // status WARN — does NOT set exit code +rep.Fail("...") // status FAIL — sets exitCode = 1 +rep.Fatal("...") // status ERR — prints + os.Exit(2) +rep.Finding("path:12 snippet") // append one detail line (repeatable) +rep.Findingf(fmt, args...) // convenience +``` + +Status **escalates** to the most severe set within a single check +(`ERR > FAIL > WARN > OK > SKIP`), so multi-branch checks (codegen, segmentation, +go-lint per-module) can report per-result and the overall verdict is the worst one. +Findings accumulate regardless of status. + +`Fatal` preserves the current hard-error behavior (checks that today call +`os.Exit(2)` when they cannot run, e.g. the Go lint pipeline failing to launch). + +All formatting lives in `render()`, so the per-check conversions cannot cause the +output format to drift — a check only chooses a status and finding strings. + +### 2. Concise render (default) + +Only checks whose status is WARN/FAIL/ERR print. Each prints a header line +` ` followed by its findings indented two spaces: + +``` +check-safety /home/alex/src/blockninja/cms +FAIL 15 No err.Error() leaked to HTTP clients + internal/service/handler.go:97 http.Error(w, err.Error(), ...) +WARN 24 any usage in changed lines (2, --all-any to scan all) + web/src/api.ts:12 [ts] data: any + +33 checks: 27 ok 3 skip 1 warn 2 fail -> FAIL +``` + +- Preamble is a single line: `check-safety <scanPath>` (replaces + `(running checks in <path>)`). Conditional plugin hint lines are preserved. +- A blank separator line precedes the summary **only** when at least one check + printed. +- Summary: `<n> checks: [<a> ok] [<b> skip] [<c> warn] [<d> fail] [<e> err] -> OK|FAIL`. + Zero counts are omitted. Verdict is `-> FAIL` when `exitCode != 0`, else `-> OK` + (warnings alone do not fail the run — unchanged from today). +- A clean run is two lines: the preamble and the summary. + +Status labels are padded to 4 columns (`OK `, `SKIP`, `WARN`, `FAIL`, `ERR `). + +`--verbose` / `-v` prints **every** check (including OK/SKIP with their summary +text), restoring the previous level of detail. + +We keep the check `id + full title` in the block header rather than inventing ~30 +short slugs: it is self-documenting and only shows on failure. + +### 3. `any` diff-scoping + +New file `changedlines.go`: + +- `newDiffIndex()` builds a lazy, per-repo cache. For a given absolute file it + locates the enclosing git repo (walk up for `.git`, cached), then once per repo + runs: + - `git -C <repo> diff --unified=0` — parse `@@ -a,b +c,d @@` hunk headers into the + set of added/modified line numbers per file. + - `git -C <repo> ls-files --others --exclude-standard` — untracked files count as + fully changed (every line). +- `func (d *diffIndex) isChanged(absPath string, line int) bool`. Files with no + enclosing repo, or a repo with no matching diff entry, return `false` (warn on + nothing). + +`checkGoAnyUsage` / `checkTypeScriptAnyUsage` gain a `keep func(absPath string, +line int) bool` parameter. When non-nil, a candidate warning is dropped unless +`keep(absPath, line)` is true. Check 2e builds `keep` from the diff index by +default; `--all-any` passes `nil` (full scan = today's behavior). The warning +headline reflects the mode: "any usage in changed lines (…, --all-any to scan all)" +vs "any usage (…)". + +Because the golden fixtures are created in fresh temp dirs that are not git repos, +default diff-mode finds no changed lines there — golden output stays deterministic. + +### 4. Flags + +Parsed in `buildScanContext`'s existing arg loop: +- `--all-any` → `ScanContext.allAny bool`. +- `--verbose` / `-v` → `Reporter.verbose bool` (set on the reporter in `main`). + +### 5. Tests / docs + +- `any_usage_test.go`: update the two `checkGoAnyUsage`/`checkTypeScriptAnyUsage` + call sites to pass `nil` (full scan). +- Regenerate both golden fixtures with `go test -run TestGoldenCharacterization + -update`. +- Update README (flag table, sample output) and `main.go`'s ordered check-list + comment where it references output shape. + +## Non-goals + +- No new per-check "slug" field. +- No change to which checks exist or their detection logic (beyond `any` scoping). +- No machine-readable/JSON output mode (could be a later addition; the collector + makes it easy).