docs: design for concise output + diff-scoped any check
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
b423f90e29
commit
3b5a6d26d6
@ -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 <dir> [--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
|
||||
`<STATUS> <id> <title>` 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).
|
||||
Loading…
x
Reference in New Issue
Block a user