From e0549987a7e6ca0cdd2ea5f9fde6c62c28e91608 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 11 Nov 2026 18:28:01 -0800 Subject: [PATCH 1/2] feat(check): per-rule summary in check --json, --summary, and one finding schema shared with verify Closes #678 --- .changeset/check-json-summary.md | 7 ++ .../.openspec.yaml | 2 + .../2026-11-11-check-json-summary/proposal.md | 29 ++++++ .../specs/cli-check/spec.md | 66 +++++++++++++ .../2026-11-11-check-json-summary/tasks.md | 21 ++++ openspec/specs/cli-check/spec.md | 71 +++++++++++--- packages/cli/README.md | 1 + packages/cli/src/agent/check.md | 32 ++++++- packages/cli/src/commands/check.ts | 32 ++++++- packages/cli/src/rules/summarize-findings.ts | 86 +++++++++++++++++ packages/cli/src/schemas/check.ts | 87 ++++++++++++----- packages/cli/src/schemas/finding.ts | 51 ++++++++++ packages/cli/src/schemas/index.ts | 19 ++++ packages/cli/src/schemas/rule-check.ts | 44 +++------ packages/cli/src/util/format.ts | 41 ++++++++ packages/cli/test/check.test.ts | 96 +++++++++++++++++++ packages/cli/test/schemas-export.test.ts | 46 +++++++++ packages/cli/test/summarize-findings.test.ts | 88 +++++++++++++++++ 18 files changed, 741 insertions(+), 78 deletions(-) create mode 100644 .changeset/check-json-summary.md create mode 100644 openspec/changes/archive/2026-11-11-check-json-summary/.openspec.yaml create mode 100644 openspec/changes/archive/2026-11-11-check-json-summary/proposal.md create mode 100644 openspec/changes/archive/2026-11-11-check-json-summary/specs/cli-check/spec.md create mode 100644 openspec/changes/archive/2026-11-11-check-json-summary/tasks.md create mode 100644 packages/cli/src/rules/summarize-findings.ts create mode 100644 packages/cli/src/schemas/finding.ts create mode 100644 packages/cli/test/summarize-findings.test.ts diff --git a/.changeset/check-json-summary.md b/.changeset/check-json-summary.md new file mode 100644 index 000000000..22e7fca14 --- /dev/null +++ b/.changeset/check-json-summary.md @@ -0,0 +1,7 @@ +--- +"@taskless/cli": patch +--- + +`check --json` now carries a `summary`: total findings, distinct files, counts by severity, and one entry per rule that fired, keyed by `engine` and `ruleId`. The new `--summary` flag returns only that summary (under `--json` it omits `results`), so an agent can measure a whole repository in one call whose output stays small; without `--json` it prints per-rule counts. Findings from `check --json` and `verify --json` now share one schema, published as `findingSchema` from `@taskless/cli/schemas` alongside `checkOutputSchema`. + +Patch: the package is 0.y.z and the change is additive. `results` and every existing key are unchanged unless you pass the new `--summary` flag, so no existing consumer has to react. diff --git a/openspec/changes/archive/2026-11-11-check-json-summary/.openspec.yaml b/openspec/changes/archive/2026-11-11-check-json-summary/.openspec.yaml new file mode 100644 index 000000000..2a7773d65 --- /dev/null +++ b/openspec/changes/archive/2026-11-11-check-json-summary/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-11 diff --git a/openspec/changes/archive/2026-11-11-check-json-summary/proposal.md b/openspec/changes/archive/2026-11-11-check-json-summary/proposal.md new file mode 100644 index 000000000..fd8045f63 --- /dev/null +++ b/openspec/changes/archive/2026-11-11-check-json-summary/proposal.md @@ -0,0 +1,29 @@ +## Why + +`check --json` has no per-rule summary, so measuring a repository means reading every finding: one agent run reported 300KB+ of output, and a harness that forbids piping to `jq` had to count by reading its saved output file. Its findings were also declared separately from the ones `verify --json` prints under `rules[].findings`, so a script written for one broke on the other (#678). + +## What Changes + +- `check --json` carries a `summary` on every run: totals, distinct files, counts by severity, and one entry per rule that fired, keyed by `engine` and `ruleId` (the pair `verify` groups `rules[]` by), most findings first. +- New `--summary` flag. With `--json` it omits `results` and keeps `summary`, so the output stays small on any repository. Without `--json` it prints per-rule counts instead of findings. The exit code is unchanged either way. +- One finding schema, `findingSchema`, declared once and used by both `check --json`'s `results[]` and `verify --json`'s `rules[].findings[]` (which extends it with `bucket`). The two had drifted: `check` allowed a `null` `note` and `fix`, `verify` did not. The shared schema takes `check`'s spelling. +- `@taskless/cli/schemas` publishes `findingSchema` and `checkOutputSchema`. +- The envelopes stay different, on purpose: `check` scans a codebase and its natural unit is a finding, while `verify` tests rules and its natural unit is a rule. `summary.rules[]` gives `check` the same per-rule grouping without moving `results`, which every existing consumer reads. +- The `check` recipe documents `summary` and `--summary` (topic v10). +- The stale JSONL description of `check --json` in the `cli-check` spec is corrected to the envelope the command has printed for a long time. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `cli-check`: `--json` output carries a per-rule summary, `--summary` returns only that, and findings share one schema with `verify`. + +## Impact + +- `packages/cli/src/schemas/finding.ts` (new), `src/schemas/check.ts`, `src/schemas/rule-check.ts`, `src/schemas/index.ts`, `src/rules/summarize-findings.ts` (new), `src/commands/check.ts`, `src/util/format.ts`, `src/agent/check.md`, `packages/cli/README.md`. +- Additive for existing `--json` consumers: `results` is unchanged unless `--summary` is passed, and `summary` is a new key. +- Delivery shape: **single PR**. The schema, the flag, the recipe and the spec only make sense together, and the diff fits one review. diff --git a/openspec/changes/archive/2026-11-11-check-json-summary/specs/cli-check/spec.md b/openspec/changes/archive/2026-11-11-check-json-summary/specs/cli-check/spec.md new file mode 100644 index 000000000..710829e42 --- /dev/null +++ b/openspec/changes/archive/2026-11-11-check-json-summary/specs/cli-check/spec.md @@ -0,0 +1,66 @@ +## REMOVED Requirements + +### Requirement: Check subcommand formats JSON output when --json is set + +**Reason**: It described a JSONL stream, one finding per line, that `check --json` has not printed for a long time. It prints one JSON object, and this change adds a key to it. + +**Migration**: Replaced by "Check subcommand prints one JSON envelope when --json is set", which states the envelope as printed. + +## ADDED Requirements + +### Requirement: Check subcommand prints one JSON envelope when --json is set + +When the `--json` flag is set, the CLI SHALL write one JSON object to stdout: `{ success, results, summary }`, plus the optional `skipped`, `failures`, `notices`, `entitlement`, `integrity` and `runDirectory` keys. `results` SHALL hold every finding, each parsing against the published `findingSchema`. `summary` SHALL be present on every run. `results` SHALL be present on every run except one given `--summary`. + +#### Scenario: JSON output carries every finding + +- **WHEN** the `--json` flag is set and the scanner produces results +- **THEN** stdout SHALL be a single JSON object whose `results` array holds one entry per finding +- **AND** its `summary` SHALL count the same findings + +#### Scenario: JSON output with no issues + +- **WHEN** the `--json` flag is set and the scanner produces zero results +- **THEN** `results` SHALL be an empty array +- **AND** `summary.total` SHALL be 0 and `summary.rules` SHALL be empty + +### Requirement: Check reports finding counts per rule + +`check --json` SHALL carry a `summary` with the total number of findings, the number of distinct files they are in, the counts by severity, and a `rules` array. Every severity SHALL be present in a count, zero included. `rules` SHALL hold one entry per rule with at least one finding, keyed by `engine` and `ruleId`, carrying that rule's total, distinct files and counts by severity, ordered by total descending and then by `ruleId`. Two engines holding the same rule id SHALL be two entries. A rule with no findings SHALL NOT be listed, because a finding list cannot tell a rule that found nothing from one that did not run. + +#### Scenario: Summary counts per rule + +- **WHEN** `check --json` runs and one error rule and one warning rule each match once in the same file +- **THEN** `summary` SHALL report a total of 2 across 1 file, with 1 error and 1 warning +- **AND** `summary.rules` SHALL hold one entry for each rule, each with a total of 1 + +#### Scenario: Summary on a project with no rules + +- **WHEN** `check --json` runs in a project with no rules configured +- **THEN** `summary` SHALL be present with a total of 0 and an empty `rules` + +### Requirement: Check --summary reports counts without findings + +`check --summary` SHALL report the run's counts in place of its findings. Under `--json` it SHALL omit `results` and keep every other key, `summary` included. Without `--json` it SHALL print one line per rule that fired, with its counts by severity and its distinct files, then a total line. The exit code SHALL be the same as without `--summary`. + +#### Scenario: JSON summary only + +- **WHEN** `check --json --summary` runs and a rule of error severity matches +- **THEN** stdout SHALL be a JSON object with `success` and `summary` and no `results` +- **AND** the process SHALL exit with code 1 + +#### Scenario: Text summary + +- **WHEN** `check --summary` runs without `--json` and one error rule and one warning rule each match once +- **THEN** stdout SHALL include a line per rule naming its engine, its id, its counts and its files +- **AND** stdout SHALL include `2 issues (1 error, 1 warning) from 2 rules across 1 file` +- **AND** stdout SHALL NOT include the matched code + +### Requirement: Check and verify share one finding schema + +A finding in `check --json`'s `results` and a finding in `verify --json`'s `rules[].findings` SHALL both parse against the one `findingSchema` published from `@taskless/cli/schemas`. `verify`'s findings SHALL extend it with `bucket` and SHALL NOT redeclare its fields. + +#### Scenario: A finding from each command parses against one schema + +- **WHEN** `check --json` and `verify --json` run against a project whose rule fires in both the code and the rule's `fail/` fixture +- **THEN** a finding from each SHALL parse against the published `findingSchema` diff --git a/openspec/changes/archive/2026-11-11-check-json-summary/tasks.md b/openspec/changes/archive/2026-11-11-check-json-summary/tasks.md new file mode 100644 index 000000000..218b33cd1 --- /dev/null +++ b/openspec/changes/archive/2026-11-11-check-json-summary/tasks.md @@ -0,0 +1,21 @@ +## 1. One finding schema + +- [x] 1.1 Declare `findingSchema` once in `schemas/finding.ts` +- [x] 1.2 Use it for `check --json`'s `results[]`, and extend it with `bucket` for `verify`'s fixture findings +- [x] 1.3 Publish `findingSchema` and `checkOutputSchema` from `@taskless/cli/schemas` + +## 2. Per-rule summary + +- [x] 2.1 Add `summarizeFindings`, grouping by engine and rule id, and the `summary` schema +- [x] 2.2 Emit `summary` on every `check --json` run, early returns included +- [x] 2.3 Add `--summary`: omit `results` under `--json`, print per-rule counts in text mode + +## 3. Docs + +- [x] 3.1 Document `summary` and `--summary` in the `check` recipe (topic v10) and the README + +## 4. Tests + +- [x] 4.1 Unit-test `summarizeFindings`: empty, ordering, file spellings, one id under two engines, an unmapped source +- [x] 4.2 Assert `summary` and `--summary` end to end, in JSON and text, and on a project with no rules +- [x] 4.3 Parse a finding from the built `check --json` and one from `verify --json` against the one published `findingSchema` diff --git a/openspec/specs/cli-check/spec.md b/openspec/specs/cli-check/spec.md index 26ce2e97e..4cb0b91a1 100644 --- a/openspec/specs/cli-check/spec.md +++ b/openspec/specs/cli-check/spec.md @@ -124,20 +124,6 @@ When `CI` is `true` or `1` and `--include-warnings` is not set, the human output - **WHEN** `CI=true` and `--json` is set - **THEN** `results` SHALL include findings of every severity -### Requirement: Check subcommand formats JSON output when --json is set - -When the `--json` flag is set, the CLI SHALL output each `CheckResult` as a JSON object per line (JSONL format) to stdout. - -#### Scenario: JSON output streams results - -- **WHEN** the `--json` flag is set and the scanner produces results -- **THEN** each `CheckResult` SHALL be written as a single-line JSON object to stdout, one per line - -#### Scenario: JSON output with no issues - -- **WHEN** the `--json` flag is set and the scanner produces zero results -- **THEN** stdout SHALL contain no output lines - ### Requirement: Check subcommand exit codes reflect error severity The CLI SHALL exit with code 0 when no error-severity matches are found (including when only warnings, info, or hints exist) and no reconcile outcome below requires failure. The CLI SHALL exit with code 1 when at least one error-severity match is found. The CLI SHALL also exit with code 1, whatever the findings and in both human and `--json` modes, when a completed reconcile: @@ -895,3 +881,60 @@ The measurement SHALL NOT change the exit code, the results, or which rules run. - **WHEN** `taskless check` runs outside CI and the latest-version lookup fails or times out - **THEN** the run SHALL complete with the results and exit code the scan alone would produce + +### Requirement: Check subcommand prints one JSON envelope when --json is set + +When the `--json` flag is set, the CLI SHALL write one JSON object to stdout: `{ success, results, summary }`, plus the optional `skipped`, `failures`, `notices`, `entitlement`, `integrity` and `runDirectory` keys. `results` SHALL hold every finding, each parsing against the published `findingSchema`. `summary` SHALL be present on every run. `results` SHALL be present on every run except one given `--summary`. + +#### Scenario: JSON output carries every finding + +- **WHEN** the `--json` flag is set and the scanner produces results +- **THEN** stdout SHALL be a single JSON object whose `results` array holds one entry per finding +- **AND** its `summary` SHALL count the same findings + +#### Scenario: JSON output with no issues + +- **WHEN** the `--json` flag is set and the scanner produces zero results +- **THEN** `results` SHALL be an empty array +- **AND** `summary.total` SHALL be 0 and `summary.rules` SHALL be empty + +### Requirement: Check reports finding counts per rule + +`check --json` SHALL carry a `summary` with the total number of findings, the number of distinct files they are in, the counts by severity, and a `rules` array. Every severity SHALL be present in a count, zero included. `rules` SHALL hold one entry per rule with at least one finding, keyed by `engine` and `ruleId`, carrying that rule's total, distinct files and counts by severity, ordered by total descending and then by `ruleId`. Two engines holding the same rule id SHALL be two entries. A rule with no findings SHALL NOT be listed, because a finding list cannot tell a rule that found nothing from one that did not run. + +#### Scenario: Summary counts per rule + +- **WHEN** `check --json` runs and one error rule and one warning rule each match once in the same file +- **THEN** `summary` SHALL report a total of 2 across 1 file, with 1 error and 1 warning +- **AND** `summary.rules` SHALL hold one entry for each rule, each with a total of 1 + +#### Scenario: Summary on a project with no rules + +- **WHEN** `check --json` runs in a project with no rules configured +- **THEN** `summary` SHALL be present with a total of 0 and an empty `rules` + +### Requirement: Check --summary reports counts without findings + +`check --summary` SHALL report the run's counts in place of its findings. Under `--json` it SHALL omit `results` and keep every other key, `summary` included. Without `--json` it SHALL print one line per rule that fired, with its counts by severity and its distinct files, then a total line. The exit code SHALL be the same as without `--summary`. + +#### Scenario: JSON summary only + +- **WHEN** `check --json --summary` runs and a rule of error severity matches +- **THEN** stdout SHALL be a JSON object with `success` and `summary` and no `results` +- **AND** the process SHALL exit with code 1 + +#### Scenario: Text summary + +- **WHEN** `check --summary` runs without `--json` and one error rule and one warning rule each match once +- **THEN** stdout SHALL include a line per rule naming its engine, its id, its counts and its files +- **AND** stdout SHALL include `2 issues (1 error, 1 warning) from 2 rules across 1 file` +- **AND** stdout SHALL NOT include the matched code + +### Requirement: Check and verify share one finding schema + +A finding in `check --json`'s `results` and a finding in `verify --json`'s `rules[].findings` SHALL both parse against the one `findingSchema` published from `@taskless/cli/schemas`. `verify`'s findings SHALL extend it with `bucket` and SHALL NOT redeclare its fields. + +#### Scenario: A finding from each command parses against one schema + +- **WHEN** `check --json` and `verify --json` run against a project whose rule fires in both the code and the rule's `fail/` fixture +- **THEN** a finding from each SHALL parse against the published `findingSchema` diff --git a/packages/cli/README.md b/packages/cli/README.md index cd9c007b3..99bbe9f64 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -53,6 +53,7 @@ pipeline: ./.taskless/bin/cli.mjs check # scan everything ./.taskless/bin/cli.mjs check $(git diff --name-only main...HEAD) # PR-only scan ./.taskless/bin/cli.mjs check --json # machine-readable +./.taskless/bin/cli.mjs check --json --summary # per-rule counts only ``` Paths that no longer exist are dropped silently, so raw `git diff` output can be diff --git a/packages/cli/src/agent/check.md b/packages/cli/src/agent/check.md index c0a01a87d..b7dd7f37a 100644 --- a/packages/cli/src/agent/check.md +++ b/packages/cli/src/agent/check.md @@ -1,4 +1,4 @@ -# Topic: check (CLI v%(CLI_VERSION)s / topic v9) +# Topic: check (CLI v%(CLI_VERSION)s / topic v10) %(ONE_COMMAND_PER_CALL)s @@ -62,7 +62,7 @@ plan does not include restoring rules, the git steps that do instead Notices about skipped runtime rules are human-readable stderr only. Under `--json` they do NOT appear as warnings; instead an additive optional `skipped: [{ rule, reason }]` array is included alongside the -unchanged `{ success, results }`, the not-verified notice is an entry +unchanged `{ success, results, summary }`, the not-verified notice is an entry in `notices`, and the CI backstop (`%(TASKLESS_CLI)s agent ci`) is the enforcement point. @@ -179,7 +179,10 @@ delete the rules to make `check` pass, and do not suggest `entitlement` with an empty `withheld` does not fail the run. ## Flags -- `--json`: machine output (`{ success, results, skipped?, failures?, notices?, integrity?, entitlement? }`). +- `--json`: machine output (`{ success, results, summary, skipped?, failures?, notices?, integrity?, entitlement? }`). +- `--summary`: per-rule counts instead of every finding. With `--json` + it omits `results` and keeps `summary`, so the output stays small on + any repository. Use it when the question is "how many", not "where". - `--anonymous`: run only static rules; skip runtime rules. - `--dangerously-run-scripts`: run runtime `check.ts` unverified. - `--timeout `: per-runtime-check wall-clock bound (default 10). @@ -249,10 +252,31 @@ delete the rules to make `check` pass, and do not suggest "matchedText": "eval(input)", "fix": null } - ] + ], + "summary": { + "total": 1, + "files": 1, + "severities": { "error": 1, "warning": 0, "info": 0, "hint": 0 }, + "rules": [ + { "engine": "sg", "ruleId": "no-eval", "total": 1, "files": 1, + "severities": { "error": 1, "warning": 0, "info": 0, "hint": 0 } } + ] + } } ``` + Each `results` entry has the same shape as a finding in + `verify --json`'s `rules[].findings` (which adds `bucket`), so one + parser reads both. `summary` is always present; its `rules` lists + only rules that fired, most findings first, keyed by `engine` and + `ruleId` as `verify` keys `rules[]`. + + **To measure a whole repository, add `--summary`.** `results` can run + to hundreds of kilobytes; `check --json --summary` returns only + `summary`, one entry per rule that fired. Do not count `results` + yourself, and do not read an absent `results` as clean: read + `summary.total`. + 4. **Report findings to the user.** Group by `file`. Show `severity`, `message`, and `ruleId` for each finding; the `range.start` is the useful line/column to surface. The `success` field reflects diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 621efd423..5cd8ea252 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -12,7 +12,7 @@ import { defineCommand } from "citty"; import { hasValeRules } from "../rules/dispatch"; import { splitRawArguments } from "../util/argv"; -import { formatText } from "../util/format"; +import { formatSummaryText, formatText } from "../util/format"; import { listRuleIds, planEngineDispatch } from "../rules/engines"; import { getTelemetry } from "../telemetry"; import { outputSchema as checkOutputSchema } from "../schemas/check"; @@ -23,6 +23,8 @@ import { requireReconciledEngines } from "../rules/engine-gate"; import { discoverRuntimeRules } from "../rules/runtime/discover"; import { resolveRuleSelection, type RuleSelection } from "../rules/rule-filter"; import { scanProject } from "../rules/scan-project"; +import { summarizeFindings } from "../rules/summarize-findings"; +import type { CheckResult } from "../types/check"; import { isCiEnvironment } from "../util/interactive"; import { markNotice } from "../util/notices"; import { @@ -262,6 +264,12 @@ export const checkCommand = defineCommand({ type: "string", description: "Run only the named rule; repeatable (--rule a --rule b)", }, + summary: { + type: "boolean", + description: + "Report finding counts per rule instead of every finding; with --json, omits `results`", + default: false, + }, "include-warnings": { type: "boolean", description: @@ -282,7 +290,7 @@ export const checkCommand = defineCommand({ // Warnings/notices are advisory human output; suppress them under --json so // the machine output stays the - // { success, results, skipped?, failures?, notices? } shape. Engine + // { success, results?, summary, skipped?, failures?, notices? } shape. Engine // failures and notices are carried in that envelope instead, since a // machine consumer cannot read stderr prose. const warn = (message: string) => { @@ -311,6 +319,15 @@ export const checkCommand = defineCommand({ for (const line of markNotice(notice, "Notice: ")) warn(line); }; + // `summary` on every `--json` run, and `results` on every run but one that + // asked for `--summary`. One place builds both, so the early returns + // below cannot disagree with a full scan about which keys a run carries. + const summaryOnly = Boolean(args.summary); + const findingsFields = (results: CheckResult[]) => ({ + ...(summaryOnly ? {} : { results }), + summary: summarizeFindings(cwd, results), + }); + // Set when a scan actually runs; drives cli_check_completed with counts // only (never matched code). let scanCounts: @@ -356,7 +373,7 @@ export const checkCommand = defineCommand({ JSON.stringify( checkOutputSchema.parse({ success: true, - results: [], + ...findingsFields([]), ...(pathNotices.length > 0 ? { notices: pathNotices } : {}), }) ) @@ -479,7 +496,7 @@ export const checkCommand = defineCommand({ JSON.stringify( checkOutputSchema.parse({ success: true, - results: [], + ...findingsFields([]), ...(pathNotices.length > 0 ? { notices: pathNotices } : {}), }) ) @@ -586,7 +603,7 @@ export const checkCommand = defineCommand({ if (args.json) { const output = checkOutputSchema.parse({ success: exitCode === 0, - results, + ...findingsFields(results), ...(plan.skipped.length > 0 ? { skipped: plan.skipped } : {}), ...(allFailures.length > 0 ? { failures: allFailures } : {}), // BOTH sources. `plan.notices` carries the repair diagnostics — @@ -602,6 +619,11 @@ export const checkCommand = defineCommand({ ...(preserve ? { runDirectory } : {}), }); console.log(JSON.stringify(output)); + } else if (summaryOnly) { + console.log(formatSummaryText(summarizeFindings(cwd, results))); + if (preserve) { + console.error(`Run directory kept: ${runDirectory}`); + } } else { // In CI, findings below error are counted rather than printed: they // never fail the run, and nobody reads them in a job log. Human diff --git a/packages/cli/src/rules/summarize-findings.ts b/packages/cli/src/rules/summarize-findings.ts new file mode 100644 index 000000000..36d70118c --- /dev/null +++ b/packages/cli/src/rules/summarize-findings.ts @@ -0,0 +1,86 @@ +import { relative, resolve } from "node:path"; + +import type { CheckSummary } from "../schemas/check"; +import type { CheckResult } from "../types/check"; +import { RUNTIME_SOURCE } from "./runtime/harness"; + +type Severity = CheckResult["severity"]; +type RuleSummary = CheckSummary["rules"][number]; +type Engine = NonNullable; + +/** The engine directory each finding `source` comes from. */ +const ENGINE_OF_SOURCE: Record = { + "ast-grep": "sg", + vale: "vale", + [RUNTIME_SOURCE]: "runtime", +}; + +function zeroSeverities(): Record { + return { error: 0, warning: 0, info: 0, hint: 0 }; +} + +/** + * Count `results` overall and per rule, for `check --json`'s `summary`. + * + * Grouped by `source` and `ruleId` together, never by id alone: two engines + * can hold a rule with the same id, and folding them into one entry would + * report a count neither rule has. Files are counted in one spelling relative + * to `cwd`, so `./a.ts` and `a.ts` are one file. + * + * Only rules with findings appear. A rule with none is not listed, because + * `results` cannot tell a rule that found nothing from one that did not run, + * and a listed zero would claim the first. + */ +export function summarizeFindings( + cwd: string, + results: readonly CheckResult[] +): CheckSummary { + const severities = zeroSeverities(); + const allFiles = new Set(); + const byRule = new Map< + string, + { summary: RuleSummary; files: Set } + >(); + + for (const result of results) { + const file = relative(cwd, resolve(cwd, result.file)); + severities[result.severity]++; + allFiles.add(file); + + const key = JSON.stringify([result.source, result.ruleId]); + let entry = byRule.get(key); + if (entry === undefined) { + const engine = ENGINE_OF_SOURCE[result.source]; + entry = { + summary: { + ...(engine === undefined ? {} : { engine }), + ruleId: result.ruleId, + total: 0, + files: 0, + severities: zeroSeverities(), + }, + files: new Set(), + }; + byRule.set(key, entry); + } + entry.summary.total++; + entry.summary.severities[result.severity]++; + entry.files.add(file); + } + + const rules = [...byRule.values()] + .map(({ summary, files }) => ({ ...summary, files: files.size })) + .toSorted( + (a, b) => + b.total - a.total || + a.ruleId.localeCompare(b.ruleId) || + (a.engine ?? "").localeCompare(b.engine ?? "") + ); + + return { + total: results.length, + files: allFiles.size, + severities, + rules, + }; +} diff --git a/packages/cli/src/schemas/check.ts b/packages/cli/src/schemas/check.ts index 05d83a82a..3aa0f3f27 100644 --- a/packages/cli/src/schemas/check.ts +++ b/packages/cli/src/schemas/check.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { findingSchema } from "./finding"; + /** A file that differs from what was issued, by signature. */ export const differingFileSchema = z.object({ path: z.string(), @@ -90,30 +92,59 @@ export const integrityEntrySchema = z.discriminatedUnion("verdict", [ /** A `check --json` `integrity` entry, typed from the schema. */ export type IntegrityEntry = z.infer; -/** Schema for a single check result */ -const checkResultSchema = z.object({ - source: z.string().describe("Scanner that produced this result"), - ruleId: z.string().describe("Rule identifier"), - severity: z - .enum(["error", "warning", "info", "hint"]) - .describe("Severity level"), - message: z.string().describe("Message explaining why the rule fired"), - note: z.string().nullable().optional().describe("Additional notes"), - file: z.string().describe("File path where the match was found"), - range: z.object({ - start: z.object({ - line: z.number(), - column: z.number(), - }), - end: z.object({ - line: z.number(), - column: z.number(), - }), - }), - matchedText: z.string().describe("The code that matched the rule"), - fix: z.string().nullable().optional().describe("Suggested fix replacement"), +/** + * Finding counts by severity. Every severity is present, zero included, so a + * reader never has to tell an absent key from a zero. + */ +const severityCountsSchema = z.strictObject({ + error: z.number().int(), + warning: z.number().int(), + info: z.number().int(), + hint: z.number().int(), +}); + +/** + * One rule's share of the findings. + * + * Grouped by `engine` and `ruleId`, the same pair `verify --json` groups + * `rules[]` by and `integrity` names a rule with, since an id alone can name a + * rule under two engines. + */ +const ruleSummarySchema = z.strictObject({ + engine: z + .enum(["sg", "vale", "runtime"]) + .optional() + .describe( + "The engine directory the rule lives under. Absent only for a finding whose `source` this CLI does not map to an engine" + ), + ruleId: z.string(), + total: z.number().int().describe("Findings this rule produced"), + files: z.number().int().describe("Distinct files those findings are in"), + severities: severityCountsSchema, }); +/** + * The per-rule measurement of a run (#678). + * + * Small whatever the repository's size: one entry per rule that fired, never + * one per finding. It is what `--summary` keeps when it drops `results`, so an + * agent can ask "how many violations of each rule" in one call whose output + * fits in a harness. + */ +export const checkSummarySchema = z.strictObject({ + total: z.number().int().describe("Findings across every rule"), + files: z.number().int().describe("Distinct files with at least one finding"), + severities: severityCountsSchema, + rules: z + .array(ruleSummarySchema) + .describe( + "One entry per rule with at least one finding, most findings first, then by `ruleId`. A rule absent from this list produced no findings, or did not run: read `skipped`, `failures` and `integrity` for the second" + ), +}); + +/** The `summary` of a `check --json` run, typed from the schema. */ +export type CheckSummary = z.infer; + /** A runtime rule that was present but not executed (advisory), for `--json`. */ const skippedRuntimeRuleSchema = z.object({ rule: z.string().describe("Runtime rule name that did not run"), @@ -123,7 +154,15 @@ const skippedRuntimeRuleSchema = z.object({ /** Output schema for `taskless check --json` on success */ export const outputSchema = z.object({ success: z.boolean(), - results: z.array(checkResultSchema).describe("Check results"), + results: z + .array(findingSchema) + .optional() + .describe( + "Every finding. Present on every run except one asked for with `--summary`, which omits it; an absent `results` therefore never means a clean run, read `summary.total`" + ), + summary: checkSummarySchema.describe( + "Finding counts, overall and per rule. Always present, `--summary` or not" + ), skipped: z .array(skippedRuntimeRuleSchema) .optional() @@ -186,5 +225,5 @@ export const outputSchema = z.object({ export const errorSchema = z.object({ success: z.literal(false), error: z.string().describe("Error message"), - results: z.array(checkResultSchema).describe("Check results (may be empty)"), + results: z.array(findingSchema).describe("Check results (may be empty)"), }); diff --git a/packages/cli/src/schemas/finding.ts b/packages/cli/src/schemas/finding.ts new file mode 100644 index 000000000..f53cbcf53 --- /dev/null +++ b/packages/cli/src/schemas/finding.ts @@ -0,0 +1,51 @@ +import { z } from "zod"; + +/** + * A position, zero-based in both axes. + */ +const positionSchema = z.object({ + line: z.number().int(), + column: z.number().int(), +}); + +/** + * One finding: a place a rule fired. + * + * The ONE finding shape, for every command that reports findings. `check + * --json` prints these as `results[]`, and `verify --json` prints them under + * `rules[].findings[]`, extended with the fixture `bucket` that produced them. + * A finding from either command parses against this schema, so a script + * written for one reads the other (#678). + * + * The two used to be declared separately and had drifted: `check` allowed a + * `null` `note` and `fix`, `verify` did not. Nothing about a finding depends on + * which command surfaced it, so there is one declaration and the looser of the + * two spellings, since `check` has always emitted it. + * + * `message` is the RENDERED message, with the rule's captures already + * interpolated. For `verify` that is the only evidence that a rule whose + * message interpolates its captures put the slots in the right order. + */ +export const findingSchema = z.object({ + source: z + .string() + .describe( + "The engine that produced it: `ast-grep`, `vale` or `taskless-runtime`" + ), + ruleId: z.string().describe("Rule identifier"), + severity: z + .enum(["error", "warning", "info", "hint"]) + .describe("Severity level"), + message: z + .string() + .describe( + "The message as the engine rendered it, with the rule's captures already interpolated" + ), + note: z.string().nullable().optional().describe("Additional notes"), + file: z + .string() + .describe("Project-relative path of the file the rule fired in"), + range: z.object({ start: positionSchema, end: positionSchema }), + matchedText: z.string().describe("The code that matched the rule"), + fix: z.string().nullable().optional().describe("Suggested fix replacement"), +}); diff --git a/packages/cli/src/schemas/index.ts b/packages/cli/src/schemas/index.ts index ed64923ea..7323effec 100644 --- a/packages/cli/src/schemas/index.ts +++ b/packages/cli/src/schemas/index.ts @@ -65,6 +65,25 @@ // layout entries: `tsc` copies this specifier verbatim into // `dist/schemas/index.d.ts`, and an extensionless one fails to resolve for a // consumer on `moduleResolution: node16`/`nodenext`. +export { + /** + * One finding, as every command that reports findings prints it: `taskless + * check --json`'s `results[]`, and `taskless verify --json`'s + * `rules[].findings[]`, which add the fixture `bucket`. A finding from either + * parses against this one schema. + */ + findingSchema, +} from "./finding.js"; + +export { + /** + * `taskless check --json`. Findings are a flat `results[]` because `check` + * scans a codebase, where `verify` reports per rule; `summary.rules[]` gives + * `check` the same per-rule grouping, keyed by `engine` and `ruleId`. + */ + outputSchema as checkOutputSchema, +} from "./check.js"; + export { /** * `taskless validate --json` and `taskless verify --json`. diff --git a/packages/cli/src/schemas/rule-check.ts b/packages/cli/src/schemas/rule-check.ts index c7b1b1bf7..e7e551c9b 100644 --- a/packages/cli/src/schemas/rule-check.ts +++ b/packages/cli/src/schemas/rule-check.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { findingSchema } from "./finding"; + /** * One rule's verdict, as `validate` and `verify` both report it. * @@ -7,43 +9,23 @@ import { z } from "zod"; * so they share one envelope. `ran` is `verify`-only: `validate` never reaches a * test run, and `verify` reports `false` when verification failed first. */ -/** - * A position, zero-based in both axes, exactly as `check --json` reports one. - */ -const positionSchema = z.object({ - line: z.number().int(), - column: z.number().int(), -}); - /** * One finding a rule's fixtures produced. * - * The `CheckResult` shape `check --json` already prints, plus the `bucket` the - * fixture that produced it lives in. Reused verbatim rather than narrowed to - * what `verify` "needs": a finding means the same thing whichever command - * surfaced it, and a second, smaller shape would be a second thing to keep in - * step with the engines. + * The shared {@link findingSchema} that `check --json` prints as `results[]`, + * plus the `bucket` the fixture that produced it lives in. Extended rather than + * redeclared: a finding means the same thing whichever command surfaced it, + * and a second declaration is a second thing to keep in step with the engines + * (the two had already drifted once, #678). * - * `message` is the field this exists for. It is the RENDERED message, with the - * rule's captures already interpolated, and it is the only evidence that a - * rule whose message interpolates its captures put the slots in the right - * order — such a rule fires on every `fail/` fixture, stays quiet on every - * `pass/` one, and a boolean verdict reports it as a rule that passed. + * `message` is the field this exists for. It is the RENDERED message, and the + * only evidence that a rule whose message interpolates its captures put the + * slots in the right order: such a rule fires on every `fail/` fixture, stays + * quiet on every `pass/` one, and a boolean verdict reports it as a rule that + * passed. */ -export const fixtureFindingSchema = z.object({ - source: z.string().describe("The engine that produced it"), - ruleId: z.string(), - severity: z.enum(["error", "warning", "info", "hint"]), - message: z - .string() - .describe( - "The message as the engine rendered it, with the rule's captures already interpolated" - ), - note: z.string().optional(), +export const fixtureFindingSchema = findingSchema.extend({ file: z.string().describe("The fixture the finding was reported against"), - range: z.object({ start: positionSchema, end: positionSchema }), - matchedText: z.string(), - fix: z.string().optional(), bucket: z .enum(["pass", "fail"]) .describe( diff --git a/packages/cli/src/util/format.ts b/packages/cli/src/util/format.ts index 35315a589..778c9a245 100644 --- a/packages/cli/src/util/format.ts +++ b/packages/cli/src/util/format.ts @@ -1,4 +1,5 @@ import type { CheckResult } from "../types/check"; +import type { CheckSummary } from "../schemas/check"; /** `2 errors, 1 warning`, in severity order, naming only the ones present. */ function describeSeverities(results: CheckResult[]): string { @@ -62,3 +63,43 @@ export function formatText( return lines.join("\n"); } + +/** `2 errors, 1 warning` from counts, in severity order, naming only nonzero. */ +function describeCounts(counts: CheckSummary["severities"]): string { + const parts: string[] = []; + for (const severity of ["error", "warning", "info", "hint"] as const) { + const count = counts[severity]; + if (count > 0) { + parts.push(`${String(count)} ${severity}${count === 1 ? "" : "s"}`); + } + } + return parts.join(", "); +} + +function plural(count: number, noun: string): string { + return `${String(count)} ${noun}${count === 1 ? "" : "s"}`; +} + +/** + * Format a run's per-rule counts, for `check --summary` without `--json`. + * + * Counts every severity, in CI too: nothing here is a finding to hide, and a + * summary that dropped the warnings would undercount the rules it lists. + */ +export function formatSummaryText(summary: CheckSummary): string { + if (summary.total === 0) return "No issues found."; + + const lines: string[] = []; + for (const rule of summary.rules) { + const name = + rule.engine === undefined ? rule.ruleId : `${rule.engine}/${rule.ruleId}`; + lines.push( + ` ${name}: ${describeCounts(rule.severities)} across ${plural(rule.files, "file")}` + ); + } + lines.push( + "", + `${plural(summary.total, "issue")} (${describeCounts(summary.severities)}) from ${plural(summary.rules.length, "rule")} across ${plural(summary.files, "file")}` + ); + return lines.join("\n"); +} diff --git a/packages/cli/test/check.test.ts b/packages/cli/test/check.test.ts index 00c952509..1683d5060 100644 --- a/packages/cli/test/check.test.ts +++ b/packages/cli/test/check.test.ts @@ -74,6 +74,11 @@ async function writeMixedSeverityProject(directory: string): Promise { ); } +/** Counts by severity, every severity present, as `summary` reports them. */ +function severities(counts: Partial>) { + return { error: 0, warning: 0, info: 0, hint: 0, ...counts }; +} + describe("check", () => { let temporaryDirectory: string; @@ -294,6 +299,97 @@ describe("check", () => { }); }); + describe("per-rule summary", () => { + it("carries a summary beside the results under --json", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout, exitCode } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + ]); + expect(exitCode).toBe(1); + const output = JSON.parse(stdout) as { + results: unknown[]; + summary: unknown; + }; + expect(output.results).toHaveLength(2); + expect(output.summary).toEqual({ + total: 2, + files: 1, + severities: severities({ error: 1, warning: 1 }), + rules: [ + { + engine: "sg", + ruleId: "no-console-error", + total: 1, + files: 1, + severities: severities({ error: 1 }), + }, + { + engine: "sg", + ruleId: "no-console-warn", + total: 1, + files: 1, + severities: severities({ warning: 1 }), + }, + ], + }); + }); + + it("omits results with --summary, and keeps the exit code", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout, exitCode } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + "--summary", + ]); + expect(exitCode).toBe(1); + const output = JSON.parse(stdout) as { + success: boolean; + summary: { total: number }; + }; + expect(output).not.toHaveProperty("results"); + expect(output.success).toBe(false); + expect(output.summary.total).toBe(2); + }); + + it("carries an empty summary when no rules are configured", async () => { + const { stdout } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + "--summary", + ]); + expect(JSON.parse(stdout)).toEqual({ + success: true, + summary: { total: 0, files: 0, severities: severities({}), rules: [] }, + }); + }); + + it("prints per-rule counts instead of findings in text mode", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--summary", + ]); + expect(stdout).toContain("sg/no-console-error: 1 error across 1 file"); + expect(stdout).toContain("sg/no-console-warn: 1 warning across 1 file"); + expect(stdout).toContain( + "2 issues (1 error, 1 warning) from 2 rules across 1 file" + ); + expect(stdout).not.toContain("console.error("); + }); + }); + it("does not require taskless.json to run", async () => { await cp(fixturesDirectory, temporaryDirectory, { recursive: true }); // Remove taskless.json if it exists — check should still work with just rules diff --git a/packages/cli/test/schemas-export.test.ts b/packages/cli/test/schemas-export.test.ts index acedfc1dc..7114db649 100644 --- a/packages/cli/test/schemas-export.test.ts +++ b/packages/cli/test/schemas-export.test.ts @@ -42,6 +42,8 @@ async function importBuiltSchemas(): Promise> { * them in the source. */ const PUBLIC_EXPORTS = [ + "checkOutputSchema", + "findingSchema", "valeVerifyOutputSchema", "verifyOutputSchema", "ruleCheckOutputSchema", @@ -151,6 +153,50 @@ describe("the published schemas entry", () => { expect(findings[0]).not.toHaveProperty("note"); }); + it("parses a check finding and a verify finding against one schema", async () => { + // #678: a script written against `check --json` broke on `verify --json`. + // Both commands now print the same finding, and the proof is a finding + // from each, emitted by the built CLI, parsed by the one published schema. + await sgProbe("probe-rule"); + await writeFile(join(cwd, "app.ts"), "eval(input);\n"); + + const check = await execFileAsync("node", [ + binPath, + "check", + "-d", + cwd, + "--json", + ]).catch((error: unknown) => error as { stdout: string }); + const verify = await execFileAsync("node", [ + binPath, + "verify", + "-d", + cwd, + "--json", + ]); + + const built = await importBuiltSchemas(); + const findingSchema = built.findingSchema as ParsedSchema; + const checkSchema = built.checkOutputSchema as ParsedSchema; + const checked = checkSchema.parse(JSON.parse(check.stdout)) as { + results: unknown[]; + summary: { rules: { engine: string; ruleId: string; total: number }[] }; + }; + const verified = JSON.parse(verify.stdout) as { + rules: { findings: unknown[] }[]; + }; + + const fromCheck = checked.results[0]; + const fromVerify = verified.rules[0]?.findings[0]; + expect(fromCheck).toBeDefined(); + expect(fromVerify).toBeDefined(); + expect(findingSchema.safeParse(fromCheck).success).toBe(true); + expect(findingSchema.safeParse(fromVerify).success).toBe(true); + expect(checked.summary.rules).toEqual([ + expect.objectContaining({ engine: "sg", ruleId: "probe-rule", total: 1 }), + ]); + }); + it("strips what it does not declare, which a JSON Schema of the same shape would not", async () => { // The reason this entry publishes zod rather than a rendering of it. A // JSON Schema validator hands back the object it was given, unknown keys diff --git a/packages/cli/test/summarize-findings.test.ts b/packages/cli/test/summarize-findings.test.ts new file mode 100644 index 000000000..ef54dab94 --- /dev/null +++ b/packages/cli/test/summarize-findings.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vitest"; + +import { checkSummarySchema } from "../src/schemas/check"; +import { summarizeFindings } from "../src/rules/summarize-findings"; +import type { CheckResult } from "../src/types/check"; + +function finding( + overrides: Partial & Pick +): CheckResult { + return { + source: "ast-grep", + severity: "error", + message: "m", + file: "a.ts", + range: { start: { line: 0, column: 0 }, end: { line: 0, column: 1 } }, + matchedText: "x", + ...overrides, + }; +} + +describe("summarizeFindings", () => { + it("is empty for no findings", () => { + expect(summarizeFindings("/repo", [])).toEqual({ + total: 0, + files: 0, + severities: { error: 0, warning: 0, info: 0, hint: 0 }, + rules: [], + }); + }); + + it("counts per rule, most findings first, files once per spelling", () => { + const summary = summarizeFindings("/repo", [ + finding({ ruleId: "b", file: "a.ts" }), + finding({ ruleId: "a", file: "a.ts", severity: "warning" }), + finding({ ruleId: "a", file: "./a.ts", severity: "hint" }), + finding({ ruleId: "a", file: "/repo/b.ts", severity: "warning" }), + ]); + + expect(checkSummarySchema.parse(summary)).toEqual(summary); + expect(summary.total).toBe(4); + expect(summary.files).toBe(2); + expect(summary.severities).toEqual({ + error: 1, + warning: 2, + info: 0, + hint: 1, + }); + expect(summary.rules).toEqual([ + { + engine: "sg", + ruleId: "a", + total: 3, + files: 2, + severities: { error: 0, warning: 2, info: 0, hint: 1 }, + }, + { + engine: "sg", + ruleId: "b", + total: 1, + files: 1, + severities: { error: 1, warning: 0, info: 0, hint: 0 }, + }, + ]); + }); + + it("keeps one id under two engines apart", () => { + const summary = summarizeFindings("/repo", [ + finding({ ruleId: "dup", source: "ast-grep" }), + finding({ ruleId: "dup", source: "vale" }), + finding({ ruleId: "dup", source: "taskless-runtime" }), + ]); + + expect( + summary.rules.map((rule) => [rule.engine, rule.ruleId, rule.total]) + ).toEqual([ + ["runtime", "dup", 1], + ["sg", "dup", 1], + ["vale", "dup", 1], + ]); + }); + + it("leaves engine out for a source it does not map", () => { + const [rule] = summarizeFindings("/repo", [ + finding({ ruleId: "x", source: "something-else" }), + ]).rules; + expect(rule).not.toHaveProperty("engine"); + }); +}); From 54ad899ea1faa35f36532352a34d74b3ebb3561a Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 11 Nov 2026 18:47:46 -0800 Subject: [PATCH 2/2] refactor(check): share one pluralization helper across text and summary output describeSeverities now counts into the zeroed severity record and delegates to describeCounts, and formatText uses plural(). Output is unchanged. --- packages/cli/src/util/format.ts | 45 ++++++++++++++------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/util/format.ts b/packages/cli/src/util/format.ts index 778c9a245..f32e86f65 100644 --- a/packages/cli/src/util/format.ts +++ b/packages/cli/src/util/format.ts @@ -1,23 +1,32 @@ import type { CheckResult } from "../types/check"; import type { CheckSummary } from "../schemas/check"; -/** `2 errors, 1 warning`, in severity order, naming only the ones present. */ -function describeSeverities(results: CheckResult[]): string { - const counts: Record = {}; - for (const result of results) { - counts[result.severity] = (counts[result.severity] ?? 0) + 1; - } +function plural(count: number, noun: string): string { + return `${String(count)} ${noun}${count === 1 ? "" : "s"}`; +} +/** `2 errors, 1 warning` from counts, in severity order, naming only nonzero. */ +function describeCounts(counts: CheckSummary["severities"]): string { const parts: string[] = []; for (const severity of ["error", "warning", "info", "hint"] as const) { const count = counts[severity]; - if (count !== undefined) { - parts.push(`${String(count)} ${severity}${count === 1 ? "" : "s"}`); - } + if (count > 0) parts.push(plural(count, severity)); } return parts.join(", "); } +/** `2 errors, 1 warning` for a list of findings; see `describeCounts`. */ +function describeSeverities(results: CheckResult[]): string { + const counts: CheckSummary["severities"] = { + error: 0, + warning: 0, + info: 0, + hint: 0, + }; + for (const result of results) counts[result.severity] += 1; + return describeCounts(counts); +} + /** * Format results as human-readable diagnostic output. * @@ -57,29 +66,13 @@ export function formatText( const uniqueFiles = new Set(results.map((r) => r.file)).size; lines.push( - `${String(results.length)} issue${results.length === 1 ? "" : "s"} (${describeSeverities(results)}) across ${String(uniqueFiles)} file${uniqueFiles === 1 ? "" : "s"}` + `${plural(results.length, "issue")} (${describeSeverities(results)}) across ${plural(uniqueFiles, "file")}` ); if (hiddenLine !== undefined) lines.push(hiddenLine); return lines.join("\n"); } -/** `2 errors, 1 warning` from counts, in severity order, naming only nonzero. */ -function describeCounts(counts: CheckSummary["severities"]): string { - const parts: string[] = []; - for (const severity of ["error", "warning", "info", "hint"] as const) { - const count = counts[severity]; - if (count > 0) { - parts.push(`${String(count)} ${severity}${count === 1 ? "" : "s"}`); - } - } - return parts.join(", "); -} - -function plural(count: number, noun: string): string { - return `${String(count)} ${noun}${count === 1 ? "" : "s"}`; -} - /** * Format a run's per-rule counts, for `check --summary` without `--json`. *