diff --git a/.github/workflows/slop-ci.yml b/.github/workflows/slop-ci.yml new file mode 100644 index 0000000..679e3dd --- /dev/null +++ b/.github/workflows/slop-ci.yml @@ -0,0 +1,155 @@ +name: Slop CI + +on: + pull_request: + paths-ignore: + - "**/*.md" + - "docs/**" + - "assets/**" + - "LICENSE" + workflow_dispatch: + inputs: + base_ref: + description: Base branch, tag, or local ref to compare against + required: false + default: main + +concurrency: + group: slop-ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + slop: + name: Slop analyzer delta + runs-on: ubuntu-latest + permissions: + contents: read + env: + SLOP_ANALYZER_VERSION: 0.1.2 + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: 1.3.10 + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Resolve base commit + id: base + env: + EVENT_NAME: ${{ github.event_name }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + INPUT_BASE_REF: ${{ inputs.base_ref }} + run: | + if [ "$EVENT_NAME" = "pull_request" ]; then + base_sha="$PR_BASE_SHA" + else + base_ref="${INPUT_BASE_REF:-main}" + if git rev-parse --verify "$base_ref^{commit}" >/dev/null 2>&1; then + base_sha="$(git rev-parse "$base_ref")" + else + git fetch --no-tags origin "$base_ref" + base_sha="$(git rev-parse FETCH_HEAD)" + fi + fi + + echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT" + + - name: Create archived base/head snapshots + id: snapshots + env: + BASE_SHA: ${{ steps.base.outputs.base_sha }} + run: | + base_dir="$(mktemp -d "${RUNNER_TEMP}/slop-base-XXXXXX")" + head_dir="$(mktemp -d "${RUNNER_TEMP}/slop-head-XXXXXX")" + + git archive "$BASE_SHA" | tar -xf - -C "$base_dir" + git archive HEAD | tar -xf - -C "$head_dir" + + echo "base_dir=$base_dir" >> "$GITHUB_OUTPUT" + echo "head_dir=$head_dir" >> "$GITHUB_OUTPUT" + + - name: Scan archived snapshots with slop-analyzer + env: + BASE_DIR: ${{ steps.snapshots.outputs.base_dir }} + HEAD_DIR: ${{ steps.snapshots.outputs.head_dir }} + run: | + mkdir -p slop-artifacts + + bunx "slop-analyzer@${SLOP_ANALYZER_VERSION}" scan "$BASE_DIR" --json > slop-artifacts/base-slop.json + bunx "slop-analyzer@${SLOP_ANALYZER_VERSION}" scan "$HEAD_DIR" --json > slop-artifacts/head-slop.json + + - name: Build Hunk review artifacts + env: + BASE_SHA: ${{ steps.base.outputs.base_sha }} + run: | + bun run scripts/slop-review.ts \ + --base slop-artifacts/base-slop.json \ + --head slop-artifacts/head-slop.json \ + > slop-artifacts/slop-summary.txt + + bun run scripts/slop-review.ts \ + --base slop-artifacts/base-slop.json \ + --head slop-artifacts/head-slop.json \ + --json \ + > slop-artifacts/slop-delta.json + + bun run scripts/slop-review.ts \ + --base slop-artifacts/base-slop.json \ + --head slop-artifacts/head-slop.json \ + --agent-context \ + > slop-artifacts/slop-agent-context.json + + git diff --no-color "$BASE_SHA"...HEAD > slop-artifacts/pr.patch + + { + echo "Review locally with Hunk:" + echo "hunk patch pr.patch --agent-context slop-agent-context.json" + } > slop-artifacts/README.txt + + - name: Publish slop summary + env: + BASE_SHA: ${{ steps.base.outputs.base_sha }} + run: | + { + echo "## Slop analyzer" + echo + echo "- analyzer: slop-analyzer@${SLOP_ANALYZER_VERSION}" + echo "- base: $BASE_SHA" + echo + echo '```text' + cat slop-artifacts/slop-summary.txt + echo '```' + echo + echo "Review locally with:" + echo '```bash' + echo 'hunk patch pr.patch --agent-context slop-agent-context.json' + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload slop review artifacts + uses: actions/upload-artifact@v4 + with: + name: slop-review + path: | + slop-artifacts/README.txt + slop-artifacts/base-slop.json + slop-artifacts/head-slop.json + slop-artifacts/slop-summary.txt + slop-artifacts/slop-delta.json + slop-artifacts/slop-agent-context.json + slop-artifacts/pr.patch + if-no-files-found: error + + - name: Fail on newly introduced slop + run: | + bun run scripts/slop-review.ts \ + --base slop-artifacts/base-slop.json \ + --head slop-artifacts/head-slop.json \ + --fail-on-new diff --git a/README.md b/README.md index 1062834..66edc93 100644 --- a/README.md +++ b/README.md @@ -209,6 +209,37 @@ hunk diff --agent-context notes.json hunk patch change.patch --agent-context notes.json ``` +#### Review `slop-analyzer` findings in Hunk + +Hunk can turn [`slop-analyzer`](https://github.com/benvinegar/slop-analyzer) JSON into inline review notes. + +For one local report: + +```bash +bunx slop-analyzer@0.1.2 scan . --json > head-slop.json +bun run scripts/slop-review.ts --head head-slop.json --agent-context > slop-agent-context.json +hunk diff --agent-context slop-agent-context.json +``` + +For CI, compare a base report to the current branch so only new findings fail the check and show up in review artifacts: + +```bash +git worktree add ../base "$BASE_SHA" +bunx slop-analyzer@0.1.2 scan ../base --json > base-slop.json +bunx slop-analyzer@0.1.2 scan . --json > head-slop.json +bun run scripts/slop-review.ts --base base-slop.json --head head-slop.json --fail-on-new +bun run scripts/slop-review.ts --base base-slop.json --head head-slop.json --agent-context > slop-agent-context.json +git diff --no-color "$BASE_SHA"...HEAD > pr.patch +``` + +The repo's PR workflow at [`.github/workflows/slop-ci.yml`](.github/workflows/slop-ci.yml) automates that flow with archived base/head snapshots and uploads the review artifacts for local Hunk inspection. + +Upload `pr.patch` plus `slop-agent-context.json` as CI artifacts, then open them locally with: + +```bash +hunk patch pr.patch --agent-context slop-agent-context.json +``` + ## Examples Ready-to-run demo diffs live in [`examples/`](examples/README.md). diff --git a/scripts/slop-review.test.ts b/scripts/slop-review.test.ts new file mode 100644 index 0000000..f4cd6a7 --- /dev/null +++ b/scripts/slop-review.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, test } from "bun:test"; +import { + buildAgentContext, + buildDeltaOccurrences, + buildDeltaReport, + formatDeltaText, + type SlopReport, +} from "./slop-review"; + +const baseReport: SlopReport = { + summary: { + findingCount: 2, + }, + findings: [ + { + ruleId: "structure.duplicate-function-signatures", + family: "structure", + severity: "medium", + scope: "file", + message: "Found 2 duplicated function signatures", + evidence: ["normalizeUser at line 1", "normalizeTeam at line 1"], + score: 3, + path: "src/users/normalize.ts", + locations: [ + { path: "src/users/normalize.ts", line: 1 }, + { path: "src/teams/normalize.ts", line: 1 }, + ], + }, + { + ruleId: "defensive.needless-try-catch", + family: "defensive", + severity: "strong", + scope: "file", + message: "Found 1 defensive try/catch block", + evidence: ["line 10: try=1, catch=1"], + score: 1, + path: "src/error.ts", + locations: [{ path: "src/error.ts", line: 10 }], + }, + ], + fileScores: [ + { path: "src/users/normalize.ts", score: 3, findingCount: 1 }, + { path: "src/teams/normalize.ts", score: 3, findingCount: 1 }, + { path: "src/error.ts", score: 1, findingCount: 1 }, + ], +}; + +const headReport: SlopReport = { + summary: { + findingCount: 3, + }, + findings: [ + ...(baseReport.findings ?? []), + { + ruleId: "structure.duplicate-function-signatures", + family: "structure", + severity: "medium", + scope: "file", + message: "Found 3 duplicated function signatures", + evidence: [ + "normalizeUser at line 1", + "normalizeTeam at line 1", + "normalizeAccount at line 1", + ], + score: 4.5, + path: "src/users/normalize.ts", + locations: [ + { path: "src/users/normalize.ts", line: 1 }, + { path: "src/teams/normalize.ts", line: 1 }, + { path: "src/accounts/normalize.ts", line: 1 }, + ], + }, + ], + fileScores: [ + { path: "src/accounts/normalize.ts", score: 4.5, findingCount: 1 }, + { path: "src/users/normalize.ts", score: 4.5, findingCount: 1 }, + { path: "src/teams/normalize.ts", score: 4.5, findingCount: 1 }, + { path: "src/error.ts", score: 1, findingCount: 1 }, + ], +}; + +describe("slop review helpers", () => { + test("delta comparison keeps only new per-file occurrences from grouped findings", () => { + const delta = buildDeltaOccurrences(baseReport, headReport); + + expect(delta).toHaveLength(3); + expect(delta.map((occurrence) => occurrence.path)).toEqual([ + "src/accounts/normalize.ts", + "src/users/normalize.ts", + "src/teams/normalize.ts", + ]); + }); + + test("agent context groups new findings by file and emits hunk-friendly annotations", () => { + const context = buildAgentContext(baseReport, headReport); + + expect(context.summary).toContain("3 new findings across 3 files vs base"); + expect(context.files.map((file) => file.path)).toEqual([ + "src/accounts/normalize.ts", + "src/users/normalize.ts", + "src/teams/normalize.ts", + ]); + expect(context.files[0]).toMatchObject({ + path: "src/accounts/normalize.ts", + summary: "1 new slop finding · hotspot score 4.50.", + annotations: [ + { + summary: "Found 3 duplicated function signatures", + newRange: [1, 1], + confidence: "medium", + source: "slop-analyzer", + author: "slop-analyzer", + }, + ], + }); + expect(context.files[0]?.annotations[0]?.rationale).toContain( + "Rule: structure.duplicate-function-signatures", + ); + expect(context.files[0]?.annotations[0]?.rationale).toContain("Also flagged in 2 other files."); + }); + + test("delta report and text summary describe new findings succinctly", () => { + const delta = buildDeltaReport(baseReport, headReport); + const text = formatDeltaText(baseReport, headReport); + + expect(delta.summary).toEqual({ + baseFindingCount: 2, + headFindingCount: 3, + newFindingCount: 3, + newFileCount: 3, + }); + expect(text).toContain("New slop findings: 3 across 3 files"); + expect(text).toContain("- src/accounts/normalize.ts"); + expect(text).toContain("medium Found 3 duplicated function signatures"); + }); +}); diff --git a/scripts/slop-review.ts b/scripts/slop-review.ts new file mode 100644 index 0000000..47be4ba --- /dev/null +++ b/scripts/slop-review.ts @@ -0,0 +1,478 @@ +#!/usr/bin/env bun + +import type { AgentAnnotation, AgentContext } from "../src/core/types"; + +type Severity = "strong" | "medium" | "weak"; + +type OutputMode = "text" | "json" | "agent-context"; + +interface SlopFindingLocation { + path: string; + line: number; + column?: number; +} + +interface SlopFinding { + ruleId: string; + family: string; + severity: Severity; + scope: string; + message: string; + evidence: string[]; + score: number; + locations: SlopFindingLocation[]; + path?: string; +} + +interface SlopFileScore { + path: string; + score: number; + findingCount: number; +} + +export interface SlopReport { + rootDir?: string; + summary?: { + findingCount?: number; + repoScore?: number; + }; + findings?: SlopFinding[]; + fileScores?: SlopFileScore[]; +} + +export interface FileFindingOccurrence { + signature: string; + path: string; + finding: SlopFinding; + locations: SlopFindingLocation[]; + primaryLocation: SlopFindingLocation; + otherFileCount: number; +} + +export interface SlopDeltaSummary { + baseFindingCount: number; + headFindingCount: number; + newFindingCount: number; + newFileCount: number; +} + +export interface SlopDeltaReport { + summary: SlopDeltaSummary; + files: Array<{ + path: string; + findings: Array<{ + ruleId: string; + severity: Severity; + message: string; + score: number; + locations: SlopFindingLocation[]; + }>; + }>; +} + +interface CliOptions { + headPath: string; + basePath?: string; + outputMode: OutputMode; + failOnNew: boolean; +} + +function usage() { + return [ + "slop-review", + "", + "Compare slop-analyzer reports, optionally fail CI on new findings, and", + "emit Hunk agent-context JSON for inline review.", + "", + "Usage:", + " bun run scripts/slop-review.ts --head [--base ] [--json|--agent-context] [--fail-on-new]", + "", + "Examples:", + " bun run scripts/slop-review.ts --head slop-report.json --agent-context > slop-agent-context.json", + " bun run scripts/slop-review.ts --base base-slop.json --head head-slop.json --fail-on-new", + " bun run scripts/slop-review.ts --base base-slop.json --head head-slop.json --json", + ].join("\n"); +} + +function compareLocations(left: SlopFindingLocation, right: SlopFindingLocation) { + return ( + left.path.localeCompare(right.path) || + left.line - right.line || + (left.column ?? 1) - (right.column ?? 1) + ); +} + +function uniqueSortedLocations(finding: SlopFinding): SlopFindingLocation[] { + const fallbackPath = finding.path ?? ""; + const locations = + finding.locations.length > 0 + ? finding.locations + : [ + { + path: fallbackPath, + line: 1, + column: 1, + }, + ]; + const seen = new Set(); + + return [...locations].sort(compareLocations).filter((location) => { + const key = `${location.path}:${location.line}:${location.column ?? 1}`; + if (seen.has(key)) { + return false; + } + + seen.add(key); + return true; + }); +} + +export function buildFileFindingOccurrences(report: SlopReport | null | undefined) { + const occurrences: FileFindingOccurrence[] = []; + + for (const finding of report?.findings ?? []) { + const locationsByPath = new Map(); + + for (const location of uniqueSortedLocations(finding)) { + const existing = locationsByPath.get(location.path) ?? []; + existing.push(location); + locationsByPath.set(location.path, existing); + } + + const otherFileCount = Math.max(0, locationsByPath.size - 1); + + for (const [path, locations] of locationsByPath.entries()) { + const primaryLocation = locations[0]; + if (!primaryLocation) { + continue; + } + + const signature = JSON.stringify({ + ruleId: finding.ruleId, + family: finding.family, + severity: finding.severity, + scope: finding.scope, + message: finding.message, + path, + evidence: finding.evidence, + locations: locations.map((location) => ({ + line: location.line, + column: location.column ?? 1, + })), + }); + + occurrences.push({ + signature, + path, + finding, + locations, + primaryLocation, + otherFileCount, + }); + } + } + + return occurrences; +} + +function fileOrderMap(report: SlopReport | null | undefined) { + return new Map((report?.fileScores ?? []).map((score, index) => [score.path, index])); +} + +function compareOccurrences( + left: FileFindingOccurrence, + right: FileFindingOccurrence, + order: Map, +) { + const leftOrder = order.get(left.path) ?? Number.MAX_SAFE_INTEGER; + const rightOrder = order.get(right.path) ?? Number.MAX_SAFE_INTEGER; + + return ( + leftOrder - rightOrder || + left.path.localeCompare(right.path) || + compareLocations(left.primaryLocation, right.primaryLocation) || + left.finding.ruleId.localeCompare(right.finding.ruleId) || + left.finding.message.localeCompare(right.finding.message) + ); +} + +export function buildDeltaOccurrences( + baseReport: SlopReport | null | undefined, + headReport: SlopReport, +) { + const baseSignatures = new Set( + buildFileFindingOccurrences(baseReport).map((occurrence) => occurrence.signature), + ); + const order = fileOrderMap(headReport); + + return buildFileFindingOccurrences(headReport) + .filter((occurrence) => !baseSignatures.has(occurrence.signature)) + .sort((left, right) => compareOccurrences(left, right, order)); +} + +function formatLocation(location: SlopFindingLocation) { + return `${location.line}:${location.column ?? 1}`; +} + +function pluralize(count: number, singular: string, plural = `${singular}s`) { + return `${count} ${count === 1 ? singular : plural}`; +} + +function scoreForPath(report: SlopReport | null | undefined, targetPath: string) { + return report?.fileScores?.find((file) => file.path === targetPath)?.score; +} + +function severityToConfidence(severity: Severity): AgentAnnotation["confidence"] { + switch (severity) { + case "strong": + return "high"; + case "medium": + return "medium"; + case "weak": + return "low"; + } +} + +function buildAnnotation(occurrence: FileFindingOccurrence, index: number): AgentAnnotation { + const extraLines = occurrence.locations.slice(1).map((location) => formatLocation(location)); + const rationaleLines = [ + `Rule: ${occurrence.finding.ruleId}`, + `Severity: ${occurrence.finding.severity}`, + `Score: ${occurrence.finding.score.toFixed(2)}`, + occurrence.otherFileCount > 0 + ? `Also flagged in ${pluralize(occurrence.otherFileCount, "other file")}.` + : null, + extraLines.length > 0 + ? `Additional flagged lines in this file: ${extraLines.join(", ")}.` + : null, + ...(occurrence.finding.evidence.length > 0 + ? ["Evidence:", ...occurrence.finding.evidence.map((evidence) => `- ${evidence}`)] + : []), + ].filter((line): line is string => Boolean(line)); + + return { + id: `slop:${occurrence.finding.ruleId}:${index}`, + newRange: [occurrence.primaryLocation.line, occurrence.primaryLocation.line], + summary: occurrence.finding.message, + rationale: rationaleLines.join("\n"), + tags: [ + "slop", + occurrence.finding.family, + occurrence.finding.severity, + occurrence.finding.ruleId, + ], + confidence: severityToConfidence(occurrence.finding.severity), + source: "slop-analyzer", + author: "slop-analyzer", + }; +} + +export function buildAgentContext( + baseReport: SlopReport | null | undefined, + headReport: SlopReport, +): AgentContext { + const deltaOccurrences = buildDeltaOccurrences(baseReport, headReport); + const occurrencesByPath = new Map(); + + for (const occurrence of deltaOccurrences) { + const existing = occurrencesByPath.get(occurrence.path) ?? []; + existing.push(occurrence); + occurrencesByPath.set(occurrence.path, existing); + } + + const files = [...occurrencesByPath.entries()].map(([path, occurrences]) => { + const score = scoreForPath(headReport, path); + return { + path, + summary: + score === undefined + ? `${pluralize(occurrences.length, "new slop finding")}.` + : `${pluralize(occurrences.length, "new slop finding")} · hotspot score ${score.toFixed(2)}.`, + annotations: occurrences.map((occurrence, index) => buildAnnotation(occurrence, index)), + }; + }); + + const baseFindingCount = baseReport?.summary?.findingCount ?? 0; + const headFindingCount = headReport.summary?.findingCount ?? 0; + + return { + version: 1, + summary: baseReport + ? `slop-analyzer found ${pluralize(deltaOccurrences.length, "new finding")} across ${pluralize(files.length, "file")} vs base (${baseFindingCount} -> ${headFindingCount} total findings).` + : `slop-analyzer found ${pluralize(deltaOccurrences.length, "finding")} across ${pluralize(files.length, "file")}.`, + files, + }; +} + +export function buildDeltaReport( + baseReport: SlopReport | null | undefined, + headReport: SlopReport, +): SlopDeltaReport { + const deltaOccurrences = buildDeltaOccurrences(baseReport, headReport); + const grouped = new Map(); + + for (const occurrence of deltaOccurrences) { + const current = grouped.get(occurrence.path) ?? { + path: occurrence.path, + findings: [], + }; + current.findings.push({ + ruleId: occurrence.finding.ruleId, + severity: occurrence.finding.severity, + message: occurrence.finding.message, + score: occurrence.finding.score, + locations: occurrence.locations, + }); + grouped.set(occurrence.path, current); + } + + return { + summary: { + baseFindingCount: baseReport?.summary?.findingCount ?? 0, + headFindingCount: headReport.summary?.findingCount ?? 0, + newFindingCount: deltaOccurrences.length, + newFileCount: grouped.size, + }, + files: [...grouped.values()], + }; +} + +export function formatDeltaText(baseReport: SlopReport | null | undefined, headReport: SlopReport) { + const delta = buildDeltaReport(baseReport, headReport); + + if (delta.summary.newFindingCount === 0) { + return baseReport + ? `No new slop findings vs base. (${delta.summary.baseFindingCount} -> ${delta.summary.headFindingCount} total findings)` + : "No slop findings."; + } + + return [ + `New slop findings: ${delta.summary.newFindingCount} across ${pluralize(delta.summary.newFileCount, "file")}`, + ...(baseReport + ? [ + `Base findings: ${delta.summary.baseFindingCount}`, + `Head findings: ${delta.summary.headFindingCount}`, + ] + : [`Head findings: ${delta.summary.headFindingCount}`]), + "", + ...delta.files.flatMap((file) => [ + `- ${file.path}`, + ...file.findings.map((finding) => { + const locations = finding.locations.map((location) => formatLocation(location)).join(", "); + return ` - ${finding.severity} ${finding.message} ${finding.ruleId} @ ${locations}`; + }), + ]), + ].join("\n"); +} + +async function readReport(pathOrDash: string) { + const text = + pathOrDash === "-" + ? await new Response(Bun.stdin.stream()).text() + : await Bun.file(pathOrDash).text(); + + return JSON.parse(text) as SlopReport; +} + +export function parseArgs(argv: string[]): CliOptions { + if (argv.includes("--help") || argv.includes("-h")) { + console.log(usage()); + process.exit(0); + } + + let headPath: string | undefined; + let basePath: string | undefined; + let outputMode: OutputMode = "text"; + let failOnNew = false; + + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (!arg) { + continue; + } + + switch (arg) { + case "--head": { + headPath = argv[index + 1]; + index += 1; + break; + } + case "--base": { + basePath = argv[index + 1]; + index += 1; + break; + } + case "--json": { + if (outputMode !== "text") { + throw new Error("Specify only one of --json or --agent-context."); + } + outputMode = "json"; + break; + } + case "--agent-context": { + if (outputMode !== "text") { + throw new Error("Specify only one of --json or --agent-context."); + } + outputMode = "agent-context"; + break; + } + case "--fail-on-new": { + failOnNew = true; + break; + } + default: { + throw new Error(`Unknown argument: ${arg}`); + } + } + } + + if (!headPath) { + throw new Error("Pass --head ."); + } + + if ( + headPath === undefined || + headPath.startsWith("--") || + (basePath?.startsWith("--") ?? false) + ) { + throw new Error("Expected a file path after --head/--base."); + } + + return { + headPath, + basePath, + outputMode, + failOnNew, + }; +} + +export async function runCli(argv: string[]) { + const options = parseArgs(argv); + const [baseReport, headReport] = await Promise.all([ + options.basePath ? readReport(options.basePath) : Promise.resolve(null), + readReport(options.headPath), + ]); + const delta = buildDeltaReport(baseReport, headReport); + + if (options.outputMode === "json") { + console.log(JSON.stringify(delta, null, 2)); + } else if (options.outputMode === "agent-context") { + console.log(JSON.stringify(buildAgentContext(baseReport, headReport), null, 2)); + } else { + console.log(formatDeltaText(baseReport, headReport)); + } + + return options.failOnNew && delta.summary.newFindingCount > 0 ? 1 : 0; +} + +if (import.meta.main) { + try { + const exitCode = await runCli(process.argv.slice(2)); + process.exit(exitCode); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + console.error(""); + console.error(usage()); + process.exit(1); + } +}