diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 0060f43..ff11cfd 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -2,5 +2,6 @@ "packages/pi-continuous-learning": "0.13.2", "packages/pi-red-green": "0.2.1", "packages/pi-compass": "0.2.0", - "packages/pi-simplify": "0.2.0" + "packages/pi-simplify": "0.2.0", + "packages/pi-code-review": "0.1.0" } diff --git a/AGENTS.md b/AGENTS.md index f7b1667..6b4a8ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,9 @@ packages/ pi-simplify/ # Pi extension: code simplification (/simplify command) src/ # TypeScript source + tests (*.test.ts alongside source) CHANGELOG.md # Release history (managed by release-please) + pi-code-review/ # Pi extension: automated language-aware code review + src/ # TypeScript source + tests (*.test.ts alongside source) + CHANGELOG.md # Release history (managed by release-please) ``` ## Commands (run from repo root) diff --git a/README.md b/README.md index aec9678..6835fd5 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ A monorepo of [Pi](https://github.com/nicholasgasior/pi-coding-agent) extensions | [pi-red-green](packages/pi-red-green) | TDD enforcement for agent sessions: RED-GREEN-REFACTOR state machine with phase-specific prompt injection and test run detection | [![npm](https://img.shields.io/npm/v/pi-red-green)](https://www.npmjs.com/package/pi-red-green) | | [pi-compass](packages/pi-compass) | Codebase navigation: generates structured codemaps and interactive code tours for faster agent onboarding | [![npm](https://img.shields.io/npm/v/pi-compass)](https://www.npmjs.com/package/pi-compass) | | [pi-simplify](packages/pi-simplify) | Code simplification: reviews recently changed files for clarity, consistency, and maintainability | [![npm](https://img.shields.io/npm/v/pi-simplify)](https://www.npmjs.com/package/pi-simplify) | +| [pi-code-review](packages/pi-code-review) | Automated code review: language-aware review after edits with structured findings | [![npm](https://img.shields.io/npm/v/pi-code-review)](https://www.npmjs.com/package/pi-code-review) | ## Development diff --git a/package-lock.json b/package-lock.json index 3dea729..50e3503 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4023,6 +4023,10 @@ "license": "MIT", "peer": true }, + "node_modules/pi-code-review": { + "resolved": "packages/pi-code-review", + "link": true + }, "node_modules/pi-compass": { "resolved": "packages/pi-compass", "link": true @@ -5002,6 +5006,27 @@ "zod": "^3.25 || ^4" } }, + "packages/pi-code-review": { + "version": "0.1.0", + "license": "MIT", + "devDependencies": { + "@types/node": "^24.0.0", + "@typescript-eslint/eslint-plugin": "^8.0.0", + "@typescript-eslint/parser": "^8.0.0", + "eslint": "^9.0.0", + "typescript": "^5.7.0", + "vitest": "^3.0.0" + }, + "engines": { + "node": ">=18" + }, + "peerDependencies": { + "@mariozechner/pi-ai": "^0.62.0", + "@mariozechner/pi-coding-agent": "^0.62.0", + "@mariozechner/pi-tui": "^0.62.0", + "@sinclair/typebox": "^0.34.0" + } + }, "packages/pi-compass": { "version": "0.2.0", "license": "MIT", diff --git a/packages/pi-code-review/README.md b/packages/pi-code-review/README.md new file mode 100644 index 0000000..6099595 --- /dev/null +++ b/packages/pi-code-review/README.md @@ -0,0 +1,41 @@ +# pi-code-review + +A [Pi](https://github.com/nicholasgasior/pi-coding-agent) extension that provides automated, language-aware code review after the agent writes or modifies files. + +## Installation + +```bash +pi install npm:pi-code-review +``` + +## Features + +### Automatic review (zero cost) + +After each turn where the agent edits files, a language-aware review checklist is injected into the system prompt. The agent self-reviews before proceeding, catching type safety issues, error handling gaps, security concerns, and naming problems. + +Supports: TypeScript, Python, Go, Rust, Java, PHP. + +### On-demand review (`/review`) + +Run a thorough code review with structured findings: + +``` +/review # review all uncommitted changes +/review --staged # only staged changes +/review --ref=main # diff against main +/review src/foo.ts # specific files +``` + +When an Anthropic API key is available, `/review` uses a direct Haiku call for structured output with severity-leveled findings (CRITICAL / HIGH / MEDIUM / INFO). Without an API key, it falls back to a prompt-based review via the session agent. + +## How it works + +1. **Edit tracking**: hooks into `tool_execution_end` to collect files modified by Write/Edit tools during each turn +2. **Turn batching**: at `turn_end`, snapshots the accumulated edits (no per-edit overhead) +3. **Prompt injection**: at `before_agent_start`, injects a brief language-specific review checklist into the system prompt +4. **On-demand**: `/review` reads file contents, calls Haiku for structured analysis, and formats findings with severity, line numbers, and suggestions + +## License + +MIT diff --git a/packages/pi-code-review/package.json b/packages/pi-code-review/package.json new file mode 100644 index 0000000..a7396dc --- /dev/null +++ b/packages/pi-code-review/package.json @@ -0,0 +1,69 @@ +{ + "name": "pi-code-review", + "version": "0.1.0", + "description": "A Pi extension that provides automated, language-aware code review after the agent writes or modifies files.", + "type": "module", + "license": "MIT", + "author": "Matt Devy", + "repository": { + "type": "git", + "url": "https://github.com/MattDevy/pi-extensions.git", + "directory": "packages/pi-code-review" + }, + "homepage": "https://github.com/MattDevy/pi-extensions/tree/main/packages/pi-code-review#readme", + "bugs": { + "url": "https://github.com/MattDevy/pi-extensions/issues" + }, + "keywords": [ + "pi-package", + "pi-extension", + "pi-coding-agent", + "code-review", + "ai", + "llm", + "ai-agent", + "coding-assistant", + "developer-tools" + ], + "engines": { + "node": ">=18" + }, + "files": [ + "dist", + "src", + "!src/**/*.test.ts", + "README.md", + "LICENSE" + ], + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "pi": { + "extensions": [ + "dist/index.js" + ] + }, + "scripts": { + "clean": "rm -rf dist tsconfig.build.tsbuildinfo", + "build": "npm run clean && tsc -p tsconfig.build.json", + "typecheck": "tsc --noEmit", + "test": "vitest run", + "lint": "eslint src/", + "check": "vitest run && eslint src/ && tsc --noEmit", + "prepublishOnly": "npm run build && npm run check", + "prepack": "test -d dist || { echo 'Error: dist/ missing. Run npm run build first.' && exit 1; }" + }, + "peerDependencies": { + "@mariozechner/pi-coding-agent": "^0.62.0", + "@mariozechner/pi-ai": "^0.62.0", + "@mariozechner/pi-tui": "^0.62.0", + "@sinclair/typebox": "^0.34.0" + }, + "devDependencies": { + "@types/node": "^24.0.0", + "@typescript-eslint/eslint-plugin": "^8.0.0", + "@typescript-eslint/parser": "^8.0.0", + "eslint": "^9.0.0", + "typescript": "^5.7.0", + "vitest": "^3.0.0" + } +} diff --git a/packages/pi-code-review/src/edit-tracker.test.ts b/packages/pi-code-review/src/edit-tracker.test.ts new file mode 100644 index 0000000..9d0b49a --- /dev/null +++ b/packages/pi-code-review/src/edit-tracker.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect } from "vitest"; +import { createEditTracker } from "./edit-tracker.js"; + +describe("createEditTracker", () => { + it("tracks Write tool edits", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { file_path: "src/foo.ts" }); + + tracker.onTurnEnd(0); + const edits = tracker.getLastTurnEdits(); + expect(edits).toEqual({ + files: [{ path: "src/foo.ts", language: "typescript" }], + turnIndex: 0, + }); + }); + + it("tracks Edit tool edits", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Edit", { file_path: "src/bar.py" }); + + tracker.onTurnEnd(1); + const edits = tracker.getLastTurnEdits(); + expect(edits).toEqual({ + files: [{ path: "src/bar.py", language: "python" }], + turnIndex: 1, + }); + }); + + it("extracts path from result.path field", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { path: "main.go" }); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()?.files).toEqual([ + { path: "main.go", language: "go" }, + ]); + }); + + it("extracts path from string result via regex", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Edit", "File: src/lib.rs updated successfully"); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()?.files).toEqual([ + { path: "src/lib.rs", language: "rust" }, + ]); + }); + + it("deduplicates files edited multiple times in a turn", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Edit", { file_path: "src/foo.ts" }); + tracker.trackEdit("Edit", { file_path: "src/foo.ts" }); + tracker.trackEdit("Write", { file_path: "src/foo.ts" }); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()?.files).toHaveLength(1); + }); + + it("tracks multiple different files", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { file_path: "src/a.ts" }); + tracker.trackEdit("Edit", { file_path: "src/b.py" }); + tracker.trackEdit("Write", { file_path: "src/c.go" }); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()?.files).toHaveLength(3); + }); + + it("ignores non-Write/Edit tools", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Bash", { stdout: "ok" }); + tracker.trackEdit("Read", { file_path: "src/foo.ts" }); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()).toBeNull(); + }); + + it("ignores non-code files", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { file_path: "package.json" }); + tracker.trackEdit("Edit", { file_path: "README.md" }); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()).toBeNull(); + }); + + it("clears accumulator after onTurnEnd", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { file_path: "src/foo.ts" }); + tracker.onTurnEnd(0); + + tracker.trackEdit("Write", { file_path: "src/bar.ts" }); + tracker.onTurnEnd(1); + + const edits = tracker.getLastTurnEdits(); + expect(edits?.files).toEqual([{ path: "src/bar.ts", language: "typescript" }]); + expect(edits?.turnIndex).toBe(1); + }); + + it("returns null when no edits in last turn", () => { + const tracker = createEditTracker(); + + tracker.onTurnEnd(0); + + expect(tracker.getLastTurnEdits()).toBeNull(); + }); + + it("clearLastTurnEdits removes snapshot", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", { file_path: "src/foo.ts" }); + tracker.onTurnEnd(0); + tracker.clearLastTurnEdits(); + + expect(tracker.getLastTurnEdits()).toBeNull(); + }); + + it("handles null/undefined result gracefully", () => { + const tracker = createEditTracker(); + + tracker.trackEdit("Write", null); + tracker.trackEdit("Edit", undefined); + + tracker.onTurnEnd(0); + expect(tracker.getLastTurnEdits()).toBeNull(); + }); +}); diff --git a/packages/pi-code-review/src/edit-tracker.ts b/packages/pi-code-review/src/edit-tracker.ts new file mode 100644 index 0000000..b397997 --- /dev/null +++ b/packages/pi-code-review/src/edit-tracker.ts @@ -0,0 +1,68 @@ +import { detectLanguage } from "./language-detector.js"; +import type { EditedFile, TurnEdits } from "./types.js"; + +const FILE_PATH_RE = /(?:File|file|path)[:\s]+(\S+)/; + +type TrackedTool = "Write" | "Edit"; +const TRACKED_TOOLS = new Set(["Write", "Edit"]); + +function extractFilePath(result: unknown): string | null { + if (!result) return null; + + if (typeof result === "object") { + const obj = result as Record; + if (typeof obj["file_path"] === "string") return obj["file_path"]; + if (typeof obj["path"] === "string") return obj["path"]; + } + + if (typeof result === "string") { + const match = result.match(FILE_PATH_RE); + return match?.[1] ?? null; + } + + return null; +} + +export interface EditTracker { + trackEdit(toolName: string, result: unknown): void; + onTurnEnd(turnIndex: number): void; + getLastTurnEdits(): TurnEdits | null; + clearLastTurnEdits(): void; +} + +export function createEditTracker(): EditTracker { + const current = new Map(); + let lastTurn: TurnEdits | null = null; + + return { + trackEdit(toolName: string, result: unknown): void { + if (!TRACKED_TOOLS.has(toolName as TrackedTool)) return; + + const path = extractFilePath(result); + if (!path) return; + + const language = detectLanguage(path); + if (!language) return; + if (current.has(path)) return; + + current.set(path, { path, language }); + }, + + onTurnEnd(turnIndex: number): void { + if (current.size === 0) { + lastTurn = null; + } else { + lastTurn = { files: [...current.values()], turnIndex }; + } + current.clear(); + }, + + getLastTurnEdits(): TurnEdits | null { + return lastTurn; + }, + + clearLastTurnEdits(): void { + lastTurn = null; + }, + }; +} diff --git a/packages/pi-code-review/src/index.test.ts b/packages/pi-code-review/src/index.test.ts new file mode 100644 index 0000000..26ace62 --- /dev/null +++ b/packages/pi-code-review/src/index.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect, vi } from "vitest"; +import registerExtension from "./index.js"; + +describe("pi-code-review extension", () => { + function makePi() { + return { + on: vi.fn(), + registerCommand: vi.fn(), + } as unknown as Parameters[0]; + } + + it("registers the review command", () => { + const pi = makePi(); + + registerExtension(pi); + + expect(pi.registerCommand).toHaveBeenCalledOnce(); + expect(pi.registerCommand).toHaveBeenCalledWith( + "review", + expect.objectContaining({ + description: expect.any(String), + handler: expect.any(Function), + }), + ); + }); + + it("registers tool_execution_end handler", () => { + const pi = makePi(); + + registerExtension(pi); + + const calls = (pi.on as ReturnType).mock.calls; + const events = calls.map((c) => c[0]); + expect(events).toContain("tool_execution_end"); + }); + + it("registers turn_end handler", () => { + const pi = makePi(); + + registerExtension(pi); + + const calls = (pi.on as ReturnType).mock.calls; + const events = calls.map((c) => c[0]); + expect(events).toContain("turn_end"); + }); + + it("registers before_agent_start handler", () => { + const pi = makePi(); + + registerExtension(pi); + + const calls = (pi.on as ReturnType).mock.calls; + const events = calls.map((c) => c[0]); + expect(events).toContain("before_agent_start"); + }); + + it("registers exactly 3 event handlers", () => { + const pi = makePi(); + + registerExtension(pi); + + expect(pi.on).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/pi-code-review/src/index.ts b/packages/pi-code-review/src/index.ts new file mode 100644 index 0000000..d672482 --- /dev/null +++ b/packages/pi-code-review/src/index.ts @@ -0,0 +1,55 @@ +import type { + ExtensionAPI, + ExtensionCommandContext, +} from "@mariozechner/pi-coding-agent"; +import { createEditTracker } from "./edit-tracker.js"; +import { + handleBeforeAgentStart, + type BeforeAgentStartEvent, +} from "./review-injector.js"; +import { handleReviewCommand, COMMAND_NAME } from "./review-command.js"; + +export default function (pi: ExtensionAPI): void { + const tracker = createEditTracker(); + + pi.on("tool_execution_end", (event, _ctx) => { + try { + const { toolName, result } = event as { + type: "tool_execution_end"; + toolName: string; + result: unknown; + }; + tracker.trackEdit(toolName, result); + } catch (err) { + console.error("[pi-code-review] tool_execution_end error:", err); + } + }); + + pi.on("turn_end", (event, _ctx) => { + try { + const { turnIndex } = event as { type: "turn_end"; turnIndex: number }; + tracker.onTurnEnd(turnIndex); + } catch (err) { + console.error("[pi-code-review] turn_end error:", err); + } + }); + + pi.on("before_agent_start", (event, _ctx) => { + try { + const lastEdits = tracker.getLastTurnEdits(); + if (!lastEdits || lastEdits.files.length === 0) return; + const result = handleBeforeAgentStart(event as BeforeAgentStartEvent, lastEdits); + tracker.clearLastTurnEdits(); + return result; + } catch (err) { + console.error("[pi-code-review] before_agent_start error:", err); + } + }); + + pi.registerCommand(COMMAND_NAME, { + description: + "Run a thorough code review on recently changed files", + handler: (args: string, ctx: ExtensionCommandContext) => + handleReviewCommand(args, ctx, pi), + }); +} diff --git a/packages/pi-code-review/src/language-detector.test.ts b/packages/pi-code-review/src/language-detector.test.ts new file mode 100644 index 0000000..fbaaee7 --- /dev/null +++ b/packages/pi-code-review/src/language-detector.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect } from "vitest"; +import { detectLanguage, getLanguageChecklist, isCodeFile } from "./language-detector.js"; + +describe("detectLanguage", () => { + it("detects TypeScript files", () => { + expect(detectLanguage("src/foo.ts")).toBe("typescript"); + expect(detectLanguage("src/bar.tsx")).toBe("typescript"); + }); + + it("detects JavaScript files as typescript", () => { + expect(detectLanguage("src/foo.js")).toBe("typescript"); + expect(detectLanguage("src/bar.jsx")).toBe("typescript"); + }); + + it("detects Python files", () => { + expect(detectLanguage("src/app.py")).toBe("python"); + }); + + it("detects Go files", () => { + expect(detectLanguage("main.go")).toBe("go"); + }); + + it("detects Rust files", () => { + expect(detectLanguage("src/lib.rs")).toBe("rust"); + }); + + it("detects Java files", () => { + expect(detectLanguage("App.java")).toBe("java"); + }); + + it("detects PHP files", () => { + expect(detectLanguage("index.php")).toBe("php"); + }); + + it("returns null for unknown extensions", () => { + expect(detectLanguage("style.css")).toBeNull(); + expect(detectLanguage("data.json")).toBeNull(); + expect(detectLanguage("README.md")).toBeNull(); + }); + + it("is case insensitive", () => { + expect(detectLanguage("App.TS")).toBe("typescript"); + expect(detectLanguage("Main.PY")).toBe("python"); + }); +}); + +describe("getLanguageChecklist", () => { + it("returns checklist for typescript", () => { + const checklist = getLanguageChecklist("typescript"); + expect(checklist.length).toBeGreaterThan(0); + expect(checklist.some((item) => item.toLowerCase().includes("type"))).toBe(true); + }); + + it("returns checklist for python", () => { + const checklist = getLanguageChecklist("python"); + expect(checklist.length).toBeGreaterThan(0); + }); + + it("returns checklist for go", () => { + const checklist = getLanguageChecklist("go"); + expect(checklist.length).toBeGreaterThan(0); + expect(checklist.some((item) => item.toLowerCase().includes("error"))).toBe(true); + }); + + it("returns empty array for unknown language", () => { + expect(getLanguageChecklist("brainfuck")).toEqual([]); + }); +}); + +describe("isCodeFile", () => { + it("returns true for code files", () => { + expect(isCodeFile("src/foo.ts")).toBe(true); + expect(isCodeFile("main.py")).toBe(true); + expect(isCodeFile("lib.rs")).toBe(true); + }); + + it("returns false for config files", () => { + expect(isCodeFile("package.json")).toBe(false); + expect(isCodeFile("tsconfig.json")).toBe(false); + expect(isCodeFile("Dockerfile")).toBe(false); + expect(isCodeFile(".eslintrc.js")).toBe(false); + }); + + it("returns false for non-code extensions", () => { + expect(isCodeFile("style.css")).toBe(false); + expect(isCodeFile("README.md")).toBe(false); + expect(isCodeFile("image.png")).toBe(false); + expect(isCodeFile("package-lock.json")).toBe(false); + }); +}); diff --git a/packages/pi-code-review/src/language-detector.ts b/packages/pi-code-review/src/language-detector.ts new file mode 100644 index 0000000..1416fa7 --- /dev/null +++ b/packages/pi-code-review/src/language-detector.ts @@ -0,0 +1,116 @@ +const LANGUAGE_MAP: Record = { + ".ts": "typescript", + ".tsx": "typescript", + ".js": "typescript", + ".jsx": "typescript", + ".py": "python", + ".go": "go", + ".rs": "rust", + ".java": "java", + ".php": "php", +}; + +const NON_CODE_EXTENSIONS = new Set([ + ".json", ".yaml", ".yml", ".toml", ".md", ".txt", + ".css", ".scss", ".html", ".svg", ".png", ".jpg", + ".lock", ".env", ".gitignore", ".editorconfig", +]); + +const CONFIG_FILENAMES = new Set([ + "package.json", "package-lock.json", + "dockerfile", "makefile", + "cargo.toml", "go.mod", "go.sum", + "build.gradle", "build.gradle.kts", "pom.xml", + "pyproject.toml", "setup.py", "setup.cfg", +]); + +const CONFIG_PREFIXES = [ + "tsconfig", "vitest.config", "jest.config", + "eslint", ".eslint", "prettier", ".prettier", +]; + +const LANGUAGE_CHECKLISTS: Record = { + typescript: [ + "Type safety: any usage, missing return types on exports, unsafe type assertions", + "Error handling: unhandled promises, missing try-catch at boundaries, swallowed errors", + "Null safety: missing optional chaining, unchecked array access, nullable values", + "Naming: unclear variable/function names, inconsistent conventions", + "Security: unsanitized input, injection vectors, hardcoded secrets", + ], + python: [ + "Type hints: missing annotations on public functions, incorrect types", + "Error handling: bare except, missing error context, swallowed exceptions", + "Resource management: missing context managers, unclosed handles", + "Naming: PEP 8 violations, unclear names", + "Security: input validation, path traversal, injection risks", + ], + go: [ + "Error handling: unchecked errors, missing error wrapping, bare returns", + "Concurrency: goroutine leaks, missing synchronization, channel misuse", + "Interface design: overly broad interfaces, unused interface methods", + "Naming: non-idiomatic names, unexported when should be exported", + "Security: input validation, SQL injection, path traversal", + ], + rust: [ + "Ownership: unnecessary clones, lifetime issues, borrow conflicts", + "Error handling: unwrap in production code, missing error context", + "Unsafe: unnecessary unsafe blocks, missing safety docs", + "Naming: non-idiomatic names, unclear module structure", + "Security: unchecked input, integer overflow, buffer issues", + ], + java: [ + "Null safety: missing null checks, Optional misuse", + "Error handling: catching Exception/Throwable, empty catch blocks", + "Concurrency: thread safety, shared mutable state", + "Naming: unclear names, convention violations", + "Security: injection risks, insecure deserialization", + ], + php: [ + "Type safety: missing type declarations, loose comparisons", + "Error handling: suppressed errors, missing exception handling", + "Security: SQL injection, XSS, CSRF, path traversal", + "Naming: PSR violations, unclear names", + "Resource management: unclosed connections, memory leaks", + ], +}; + +function getExtension(filePath: string): string { + const dot = filePath.lastIndexOf("."); + return dot === -1 ? "" : filePath.slice(dot).toLowerCase(); +} + +export function detectLanguage(filePath: string): string | null { + return LANGUAGE_MAP[getExtension(filePath)] ?? null; +} + +export function getLanguageChecklist(language: string): readonly string[] { + return LANGUAGE_CHECKLISTS[language] ?? []; +} + +export function buildLanguageSection(languages: readonly string[]): string { + const parts: string[] = []; + for (const lang of languages) { + const items = getLanguageChecklist(lang); + if (items.length > 0) { + const formatted = items.map((item) => `- ${item}`).join("\n"); + parts.push(`### ${lang[0]!.toUpperCase()}${lang.slice(1)}\n${formatted}`); + } + } + return parts.length > 0 ? "\n\n" + parts.join("\n\n") : ""; +} + +export function isCodeFile(filePath: string): boolean { + const normalized = filePath.replace(/\\/g, "/"); + const segments = normalized.split("/"); + const filename = segments[segments.length - 1] ?? ""; + const lower = filename.toLowerCase(); + const ext = getExtension(filePath); + + if (CONFIG_FILENAMES.has(lower)) return false; + for (const prefix of CONFIG_PREFIXES) { + if (lower.startsWith(prefix)) return false; + } + if (NON_CODE_EXTENSIONS.has(ext)) return false; + + return LANGUAGE_MAP[ext] !== undefined; +} diff --git a/packages/pi-code-review/src/review-command.test.ts b/packages/pi-code-review/src/review-command.test.ts new file mode 100644 index 0000000..8c039d4 --- /dev/null +++ b/packages/pi-code-review/src/review-command.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, vi } from "vitest"; +import { handleReviewCommand, parseArgs, COMMAND_NAME } from "./review-command.js"; + +describe("COMMAND_NAME", () => { + it("is 'review'", () => { + expect(COMMAND_NAME).toBe("review"); + }); +}); + +describe("parseArgs", () => { + it("returns defaults for empty string", () => { + expect(parseArgs("")).toEqual({ files: [], ref: "HEAD", staged: false }); + }); + + it("parses --staged flag", () => { + expect(parseArgs("--staged")).toEqual({ files: [], ref: "HEAD", staged: true }); + }); + + it("parses --ref=", () => { + expect(parseArgs("--ref=main")).toEqual({ files: [], ref: "main", staged: false }); + }); + + it("parses file paths", () => { + expect(parseArgs("src/a.ts src/b.ts")).toEqual({ + files: ["src/a.ts", "src/b.ts"], + ref: "HEAD", + staged: false, + }); + }); + + it("parses mix of flags and paths", () => { + expect(parseArgs("--staged src/a.ts")).toEqual({ + files: ["src/a.ts"], + ref: "HEAD", + staged: true, + }); + }); +}); + +describe("handleReviewCommand", () => { + function makeMocks(options: { + changedFiles?: string[]; + fileContents?: Record; + apiKey?: string | undefined; + llmResponse?: string; + }) { + const { + changedFiles = [], + fileContents = {}, + apiKey = undefined, + llmResponse = '{"findings": []}', + } = options; + + const diffStdout = changedFiles.map((f) => `M\t${f}`).join("\n") + "\n"; + + const pi = { + exec: vi.fn((_cmd: string, args: string[]) => { + if (args[0] === "diff") { + if (changedFiles.length > 0) { + return Promise.resolve({ stdout: diffStdout, stderr: "", code: 0 }); + } + return Promise.resolve({ stdout: "", stderr: "", code: 1 }); + } + // cat command for reading files + const filePath = args[args.length - 1] ?? ""; + const content = fileContents[filePath] ?? "file content"; + return Promise.resolve({ stdout: content, stderr: "", code: 0 }); + }), + sendUserMessage: vi.fn(), + } as unknown as Parameters[2]; + + const ctx = { + cwd: "/project", + ui: { notify: vi.fn() }, + modelRegistry: { + getApiKeyForProvider: vi.fn(() => Promise.resolve(apiKey)), + }, + } as unknown as Parameters[1]; + + // Mock the complete function via the module + const mockComplete = vi.fn(() => + Promise.resolve({ + content: [{ type: "text", text: llmResponse }], + }), + ); + + return { pi, ctx, mockComplete }; + } + + it("notifies when no changed files found", async () => { + const { pi, ctx } = makeMocks({}); + + await handleReviewCommand("", ctx, pi); + + expect(pi.sendUserMessage).not.toHaveBeenCalled(); + expect(ctx.ui.notify).toHaveBeenCalledWith( + expect.stringContaining("No changed files"), + "info", + ); + }); + + it("falls back to prompt-only when no API key", async () => { + const { pi, ctx } = makeMocks({ + changedFiles: ["src/foo.ts"], + apiKey: undefined, + }); + + await handleReviewCommand("", ctx, pi); + + expect(pi.sendUserMessage).toHaveBeenCalledOnce(); + const prompt = (pi.sendUserMessage as ReturnType).mock.calls[0]?.[0] as string; + expect(prompt).toContain("src/foo.ts"); + expect(prompt).toContain("CRITICAL"); + }); + + it("uses explicit file paths from args without git", async () => { + const { pi, ctx } = makeMocks({ apiKey: undefined }); + + await handleReviewCommand("src/a.ts src/b.ts", ctx, pi); + + expect(pi.sendUserMessage).toHaveBeenCalledOnce(); + const prompt = (pi.sendUserMessage as ReturnType).mock.calls[0]?.[0] as string; + expect(prompt).toContain("src/a.ts"); + expect(prompt).toContain("src/b.ts"); + }); + + it("sends with deliverAs followUp", async () => { + const { pi, ctx } = makeMocks({ + changedFiles: ["src/foo.ts"], + apiKey: undefined, + }); + + await handleReviewCommand("", ctx, pi); + + expect(pi.sendUserMessage).toHaveBeenCalledWith( + expect.any(String), + { deliverAs: "followUp" }, + ); + }); +}); diff --git a/packages/pi-code-review/src/review-command.ts b/packages/pi-code-review/src/review-command.ts new file mode 100644 index 0000000..242158d --- /dev/null +++ b/packages/pi-code-review/src/review-command.ts @@ -0,0 +1,174 @@ +import type { + ExtensionAPI, + ExtensionCommandContext, +} from "@mariozechner/pi-coding-agent"; +import { complete, getModel } from "@mariozechner/pi-ai"; +import { detectLanguage } from "./language-detector.js"; +import { buildReviewPrompt, buildFallbackPrompt } from "./review-prompt.js"; +import { parseReviewFindings, formatFindings } from "./review-parser.js"; +import type { EditedFile } from "./types.js"; + +export const COMMAND_NAME = "review"; + +const HAIKU_MODEL_ID = "claude-haiku-4-5-20251001"; +const MAX_FILES = 10; + +interface ReviewOptions { + readonly files: readonly string[]; + readonly ref: string; + readonly staged: boolean; +} + +export function parseArgs(args: string): ReviewOptions { + const tokens = args.trim().split(/\s+/).filter(Boolean); + const files: string[] = []; + let ref = "HEAD"; + let staged = false; + + for (const token of tokens) { + if (token === "--staged") { + staged = true; + } else if (token.startsWith("--ref=")) { + ref = token.slice("--ref=".length); + } else { + files.push(token); + } + } + + return { files, ref, staged }; +} + +interface ChangedFile { + readonly path: string; + readonly status: string; +} + +const STATUS_MAP: Record = { + M: "modified", + A: "added", + R: "renamed", + C: "copied", +}; + +function parseDiffOutput(stdout: string): ChangedFile[] { + const files: ChangedFile[] = []; + for (const line of stdout.split("\n")) { + if (!line.trim()) continue; + const parts = line.split("\t"); + const statusCode = parts[0]?.[0]; + if (!statusCode) continue; + const status = STATUS_MAP[statusCode]; + if (!status) continue; + const path = (statusCode === "R" || statusCode === "C") ? parts[2] : parts[1]; + if (path) files.push({ path, status }); + } + return files; +} + +async function getChangedFiles( + pi: ExtensionAPI, + cwd: string, + options: ReviewOptions, +): Promise { + if (options.files.length > 0) { + return [...options.files]; + } + + const args = ["diff", "--name-status"]; + if (options.staged) { + args.push("--cached"); + } else { + args.push(options.ref); + } + + const result = await pi.exec("git", args, { cwd }); + if (result.code === 0) { + const files = parseDiffOutput(result.stdout).map((f) => f.path); + if (files.length > 0) return files; + } + + const fallback = await pi.exec("git", ["diff", "--name-status", "HEAD~1"], { cwd }); + if (fallback.code === 0) { + return parseDiffOutput(fallback.stdout).map((f) => f.path); + } + + return []; +} + +async function readFileContent( + pi: ExtensionAPI, + cwd: string, + filePath: string, +): Promise { + const result = await pi.exec("cat", [filePath], { cwd }); + if (result.code === 0) return result.stdout; + return ""; +} + +export async function handleReviewCommand( + args: string, + ctx: ExtensionCommandContext, + pi: ExtensionAPI, +): Promise { + const options = parseArgs(args); + const filePaths = await getChangedFiles(pi, ctx.cwd, options); + + if (filePaths.length === 0) { + ctx.ui.notify( + "No changed files found. Specify file paths or make some changes first.", + "info", + ); + return; + } + + const capped = filePaths.slice(0, MAX_FILES); + if (filePaths.length > MAX_FILES) { + ctx.ui.notify( + `Reviewing first ${MAX_FILES} of ${filePaths.length} changed files.`, + "info", + ); + } + + const editedFiles: EditedFile[] = capped.map((path) => ({ + path, + language: detectLanguage(path), + })); + + const apiKey = await ctx.modelRegistry.getApiKeyForProvider("anthropic"); + if (apiKey) { + try { + const filesWithContent = await Promise.all( + editedFiles.map(async (f) => ({ + ...f, + content: await readFileContent(pi, ctx.cwd, f.path), + })), + ); + + const prompt = buildReviewPrompt(filesWithContent); + const model = getModel("anthropic", HAIKU_MODEL_ID); + const context = { + messages: [{ role: "user" as const, content: prompt, timestamp: Date.now() }], + }; + + const message = await complete(model, context, { apiKey }); + const text = message.content + .filter((c) => c.type === "text") + .map((c) => (c as { type: "text"; text: string }).text) + .join(""); + + const findings = parseReviewFindings(text); + const formatted = formatFindings(findings); + + pi.sendUserMessage( + `Code review complete for ${capped.length} file(s):\n\n${formatted}`, + { deliverAs: "followUp" }, + ); + return; + } catch (err) { + console.warn("[pi-code-review] Haiku review failed, falling back to prompt:", err); + } + } + + const fallbackPrompt = buildFallbackPrompt(editedFiles); + pi.sendUserMessage(fallbackPrompt, { deliverAs: "followUp" }); +} diff --git a/packages/pi-code-review/src/review-injector.test.ts b/packages/pi-code-review/src/review-injector.test.ts new file mode 100644 index 0000000..ce2cacc --- /dev/null +++ b/packages/pi-code-review/src/review-injector.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect } from "vitest"; +import { + buildReviewInjectionBlock, + handleBeforeAgentStart, +} from "./review-injector.js"; +import type { TurnEdits } from "./types.js"; + +describe("buildReviewInjectionBlock", () => { + it("includes file paths in the injection", () => { + const edits: TurnEdits = { + files: [ + { path: "src/foo.ts", language: "typescript" }, + { path: "src/bar.ts", language: "typescript" }, + ], + turnIndex: 0, + }; + + const block = buildReviewInjectionBlock(edits); + + expect(block).toContain("src/foo.ts"); + expect(block).toContain("src/bar.ts"); + }); + + it("includes language-specific checklist items", () => { + const edits: TurnEdits = { + files: [{ path: "src/foo.ts", language: "typescript" }], + turnIndex: 0, + }; + + const block = buildReviewInjectionBlock(edits); + + expect(block).toMatch(/type safety/i); + expect(block).toMatch(/error handling/i); + }); + + it("includes checklists for multiple languages", () => { + const edits: TurnEdits = { + files: [ + { path: "src/foo.ts", language: "typescript" }, + { path: "main.py", language: "python" }, + ], + turnIndex: 0, + }; + + const block = buildReviewInjectionBlock(edits); + + expect(block).toMatch(/typescript/i); + expect(block).toMatch(/python/i); + }); + + it("handles files with null language", () => { + const edits: TurnEdits = { + files: [{ path: "src/unknown.xyz", language: null }], + turnIndex: 0, + }; + + const block = buildReviewInjectionBlock(edits); + + expect(block).toContain("src/unknown.xyz"); + }); + + it("returns null for empty files list", () => { + const edits: TurnEdits = { files: [], turnIndex: 0 }; + + expect(buildReviewInjectionBlock(edits)).toBeNull(); + }); +}); + +describe("handleBeforeAgentStart", () => { + it("appends injection block to system prompt", () => { + const event = { + type: "before_agent_start" as const, + prompt: "user prompt", + systemPrompt: "base system prompt", + }; + const edits: TurnEdits = { + files: [{ path: "src/foo.ts", language: "typescript" }], + turnIndex: 0, + }; + + const result = handleBeforeAgentStart(event, edits); + + expect(result).toBeDefined(); + expect(result?.systemPrompt).toContain("base system prompt"); + expect(result?.systemPrompt).toContain("src/foo.ts"); + }); + + it("returns undefined for empty edits", () => { + const event = { + type: "before_agent_start" as const, + prompt: "user prompt", + systemPrompt: "base system prompt", + }; + const edits: TurnEdits = { files: [], turnIndex: 0 }; + + expect(handleBeforeAgentStart(event, edits)).toBeUndefined(); + }); +}); diff --git a/packages/pi-code-review/src/review-injector.ts b/packages/pi-code-review/src/review-injector.ts new file mode 100644 index 0000000..6a2d9c2 --- /dev/null +++ b/packages/pi-code-review/src/review-injector.ts @@ -0,0 +1,40 @@ +import { buildLanguageSection } from "./language-detector.js"; +import type { TurnEdits } from "./types.js"; + +export interface BeforeAgentStartEvent { + type: "before_agent_start"; + prompt: string; + systemPrompt: string; +} + +export interface InjectionResult { + systemPrompt: string; +} + +export function buildReviewInjectionBlock(edits: TurnEdits): string | null { + if (edits.files.length === 0) return null; + + const fileList = edits.files.map((f) => f.path).join(", "); + const languages = [ + ...new Set( + edits.files.map((f) => f.language).filter((l): l is string => l !== null), + ), + ]; + + const checklist = buildLanguageSection(languages); + + return `\n\n## Code Review + +You edited: ${fileList}. Before proceeding, briefly verify:${checklist} + +Fix any issues found, then continue.`; +} + +export function handleBeforeAgentStart( + event: BeforeAgentStartEvent, + edits: TurnEdits, +): InjectionResult | undefined { + const block = buildReviewInjectionBlock(edits); + if (!block) return undefined; + return { systemPrompt: event.systemPrompt + block }; +} diff --git a/packages/pi-code-review/src/review-parser.test.ts b/packages/pi-code-review/src/review-parser.test.ts new file mode 100644 index 0000000..13c7395 --- /dev/null +++ b/packages/pi-code-review/src/review-parser.test.ts @@ -0,0 +1,143 @@ +import { describe, it, expect } from "vitest"; +import { parseReviewFindings, formatFindings } from "./review-parser.js"; + +describe("parseReviewFindings", () => { + it("parses valid JSON findings", () => { + const text = JSON.stringify({ + findings: [ + { + severity: "HIGH", + file: "src/foo.ts", + line: 42, + category: "error-handling", + message: "Unhandled promise rejection", + suggestion: "Add try-catch", + }, + ], + }); + + const findings = parseReviewFindings(text); + + expect(findings).toHaveLength(1); + expect(findings[0]).toEqual({ + severity: "HIGH", + file: "src/foo.ts", + line: 42, + category: "error-handling", + message: "Unhandled promise rejection", + suggestion: "Add try-catch", + }); + }); + + it("strips markdown code fences", () => { + const text = "```json\n" + JSON.stringify({ + findings: [ + { severity: "MEDIUM", file: "a.ts", category: "naming", message: "Unclear name" }, + ], + }) + "\n```"; + + const findings = parseReviewFindings(text); + + expect(findings).toHaveLength(1); + expect(findings[0]?.severity).toBe("MEDIUM"); + }); + + it("handles empty findings array", () => { + const text = JSON.stringify({ findings: [] }); + + expect(parseReviewFindings(text)).toEqual([]); + }); + + it("filters out invalid findings", () => { + const text = JSON.stringify({ + findings: [ + { severity: "HIGH", file: "a.ts", category: "bug", message: "Valid" }, + { severity: "INVALID", file: "b.ts", category: "bug", message: "Bad severity" }, + { file: "c.ts", category: "bug", message: "Missing severity" }, + { severity: "LOW", message: "Missing file" }, + ], + }); + + const findings = parseReviewFindings(text); + + expect(findings).toHaveLength(1); + expect(findings[0]?.file).toBe("a.ts"); + }); + + it("returns fallback finding for unparseable text", () => { + const findings = parseReviewFindings("This is not JSON at all"); + + expect(findings).toHaveLength(1); + expect(findings[0]?.severity).toBe("INFO"); + expect(findings[0]?.category).toBe("parse-error"); + }); + + it("handles JSON without findings key", () => { + const text = JSON.stringify({ results: [] }); + + const findings = parseReviewFindings(text); + + expect(findings).toHaveLength(1); + expect(findings[0]?.category).toBe("parse-error"); + }); + + it("omits optional fields when not present", () => { + const text = JSON.stringify({ + findings: [ + { severity: "INFO", file: "a.ts", category: "style", message: "Minor" }, + ], + }); + + const findings = parseReviewFindings(text); + + expect(findings[0]?.line).toBeUndefined(); + expect(findings[0]?.suggestion).toBeUndefined(); + }); +}); + +describe("formatFindings", () => { + it("formats findings as markdown", () => { + const output = formatFindings([ + { + severity: "HIGH", + file: "src/foo.ts", + line: 10, + category: "security", + message: "SQL injection risk", + suggestion: "Use parameterized queries", + }, + { + severity: "INFO", + file: "src/bar.ts", + category: "naming", + message: "Unclear variable name", + }, + ]); + + expect(output).toContain("HIGH"); + expect(output).toContain("src/foo.ts"); + expect(output).toContain("SQL injection risk"); + expect(output).toContain("parameterized queries"); + }); + + it("returns clean message when no findings", () => { + const output = formatFindings([]); + + expect(output).toMatch(/no.*issues|clean/i); + }); + + it("groups by severity", () => { + const output = formatFindings([ + { severity: "CRITICAL", file: "a.ts", category: "sec", message: "Bad" }, + { severity: "INFO", file: "b.ts", category: "style", message: "Meh" }, + { severity: "HIGH", file: "c.ts", category: "bug", message: "Oops" }, + ]); + + const criticalIdx = output.indexOf("CRITICAL"); + const highIdx = output.indexOf("HIGH"); + const infoIdx = output.indexOf("INFO"); + + expect(criticalIdx).toBeLessThan(highIdx); + expect(highIdx).toBeLessThan(infoIdx); + }); +}); diff --git a/packages/pi-code-review/src/review-parser.ts b/packages/pi-code-review/src/review-parser.ts new file mode 100644 index 0000000..05375f7 --- /dev/null +++ b/packages/pi-code-review/src/review-parser.ts @@ -0,0 +1,104 @@ +import type { ReviewFinding, ReviewSeverity } from "./types.js"; + +const VALID_SEVERITIES = new Set(["CRITICAL", "HIGH", "MEDIUM", "INFO"]); + +const SEVERITY_ORDER: Record = { + CRITICAL: 0, + HIGH: 1, + MEDIUM: 2, + INFO: 3, +}; + +function stripCodeFences(text: string): string { + return text.replace(/^```(?:json)?\s*\n?/gm, "").replace(/\n?```\s*$/gm, ""); +} + +function isValidFinding(obj: Record): boolean { + return ( + typeof obj["severity"] === "string" && + VALID_SEVERITIES.has(obj["severity"]) && + typeof obj["file"] === "string" && + typeof obj["category"] === "string" && + typeof obj["message"] === "string" + ); +} + +function toFinding(item: Record): ReviewFinding { + return { + severity: item["severity"] as ReviewSeverity, + file: item["file"] as string, + category: item["category"] as string, + message: item["message"] as string, + ...(typeof item["line"] === "number" ? { line: item["line"] } : {}), + ...(typeof item["suggestion"] === "string" ? { suggestion: item["suggestion"] } : {}), + }; +} + +export function parseReviewFindings(text: string): ReviewFinding[] { + const cleaned = stripCodeFences(text.trim()); + + try { + const parsed: unknown = JSON.parse(cleaned); + if (!parsed || typeof parsed !== "object") { + return makeFallback(text); + } + + const obj = parsed as Record; + if (!Array.isArray(obj["findings"])) { + return makeFallback(text); + } + + const findings: ReviewFinding[] = []; + for (const item of obj["findings"] as unknown[]) { + const raw = item as Record; + if (isValidFinding(raw)) { + findings.push(toFinding(raw)); + } + } + return findings; + } catch { + return makeFallback(text); + } +} + +function makeFallback(text: string): ReviewFinding[] { + return [ + { + severity: "INFO", + file: "", + category: "parse-error", + message: `Could not parse structured review. Raw response: ${text.slice(0, 200)}`, + }, + ]; +} + +export function formatFindings(findings: readonly ReviewFinding[]): string { + if (findings.length === 0) { + return "Code review complete: no issues found. Code looks clean."; + } + + const sorted = [...findings].sort( + (a, b) => SEVERITY_ORDER[a.severity] - SEVERITY_ORDER[b.severity], + ); + + const lines = sorted.map((f) => { + const location = f.line ? `${f.file}:${f.line}` : f.file; + const suggestion = f.suggestion ? `\n Suggestion: ${f.suggestion}` : ""; + return `- **${f.severity}** [${f.category}] ${location}: ${f.message}${suggestion}`; + }); + + const counts = findings.reduce( + (acc, f) => { + acc[f.severity] = (acc[f.severity] ?? 0) + 1; + return acc; + }, + {} as Record, + ); + + const summary = Object.entries(counts) + .sort(([a], [b]) => SEVERITY_ORDER[a as ReviewSeverity] - SEVERITY_ORDER[b as ReviewSeverity]) + .map(([sev, count]) => `${count} ${sev}`) + .join(", "); + + return `## Code Review Findings (${summary})\n\n${lines.join("\n")}`; +} diff --git a/packages/pi-code-review/src/review-prompt.test.ts b/packages/pi-code-review/src/review-prompt.test.ts new file mode 100644 index 0000000..a6b5311 --- /dev/null +++ b/packages/pi-code-review/src/review-prompt.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from "vitest"; +import { buildReviewPrompt, buildFallbackPrompt } from "./review-prompt.js"; + +describe("buildReviewPrompt", () => { + const files = [ + { path: "src/foo.ts", content: "const x = 1;", language: "typescript" as const }, + { path: "main.py", content: "x = 1", language: "python" as const }, + ]; + + it("includes file paths", () => { + const prompt = buildReviewPrompt(files); + + expect(prompt).toContain("src/foo.ts"); + expect(prompt).toContain("main.py"); + }); + + it("includes file contents", () => { + const prompt = buildReviewPrompt(files); + + expect(prompt).toContain("const x = 1;"); + expect(prompt).toContain("x = 1"); + }); + + it("requests JSON output", () => { + const prompt = buildReviewPrompt(files); + + expect(prompt).toContain("JSON"); + expect(prompt).toContain("findings"); + }); + + it("includes severity levels", () => { + const prompt = buildReviewPrompt(files); + + expect(prompt).toContain("CRITICAL"); + expect(prompt).toContain("HIGH"); + expect(prompt).toContain("MEDIUM"); + expect(prompt).toContain("INFO"); + }); + + it("includes language-specific focus areas", () => { + const prompt = buildReviewPrompt(files); + + expect(prompt).toMatch(/typescript/i); + expect(prompt).toMatch(/python/i); + }); + + it("truncates long file content", () => { + const longContent = "x".repeat(5000); + const prompt = buildReviewPrompt([ + { path: "big.ts", content: longContent, language: "typescript" }, + ]); + + expect(prompt).not.toContain("x".repeat(5000)); + expect(prompt).toContain("[truncated]"); + }); + + it("handles files with null language", () => { + const prompt = buildReviewPrompt([ + { path: "unknown.xyz", content: "data", language: null }, + ]); + + expect(prompt).toContain("unknown.xyz"); + }); +}); + +describe("buildFallbackPrompt", () => { + it("includes file paths", () => { + const prompt = buildFallbackPrompt([ + { path: "src/foo.ts", language: "typescript" }, + { path: "src/bar.py", language: "python" }, + ]); + + expect(prompt).toContain("src/foo.ts"); + expect(prompt).toContain("src/bar.py"); + }); + + it("includes severity format instructions", () => { + const prompt = buildFallbackPrompt([ + { path: "src/foo.ts", language: "typescript" }, + ]); + + expect(prompt).toContain("CRITICAL"); + expect(prompt).toContain("HIGH"); + }); + + it("includes language checklists", () => { + const prompt = buildFallbackPrompt([ + { path: "src/foo.ts", language: "typescript" }, + ]); + + expect(prompt).toMatch(/type safety/i); + }); +}); diff --git a/packages/pi-code-review/src/review-prompt.ts b/packages/pi-code-review/src/review-prompt.ts new file mode 100644 index 0000000..f6b5235 --- /dev/null +++ b/packages/pi-code-review/src/review-prompt.ts @@ -0,0 +1,81 @@ +import { buildLanguageSection } from "./language-detector.js"; +import type { EditedFile } from "./types.js"; + +const MAX_FILE_CHARS = 4000; + +interface FileWithContent { + readonly path: string; + readonly content: string; + readonly language: string | null; +} + +function truncate(content: string, max: number): string { + if (content.length <= max) return content; + return content.slice(0, max) + "\n[truncated]"; +} + +export function buildReviewPrompt(files: readonly FileWithContent[]): string { + const languages = [ + ...new Set( + files.map((f) => f.language).filter((l): l is string => l !== null), + ), + ]; + + const fileBlocks = files + .map( + (f) => + `### ${f.path}${f.language ? ` (${f.language})` : ""}\n\`\`\`\n${truncate(f.content, MAX_FILE_CHARS)}\n\`\`\``, + ) + .join("\n\n"); + + const langSection = buildLanguageSection(languages); + + return `Review the following files for code quality issues. Return your findings as JSON in this exact format: + +\`\`\`json +{"findings": [{"severity": "CRITICAL|HIGH|MEDIUM|INFO", "file": "path", "line": 42, "category": "category", "message": "description", "suggestion": "fix"}]} +\`\`\` + +Severity guide: +- CRITICAL: Security vulnerabilities, data loss risks, crash-inducing bugs +- HIGH: Bugs, unhandled errors, type safety violations +- MEDIUM: Anti-patterns, maintainability issues, naming problems +- INFO: Style improvements, minor optimizations + +Focus areas by language:${langSection} + +If no issues are found, return: \`{"findings": []}\` + +## Files to Review + +${fileBlocks}`; +} + +export function buildFallbackPrompt(files: readonly EditedFile[]): string { + const languages = [ + ...new Set( + files.map((f) => f.language).filter((l): l is string => l !== null), + ), + ]; + + const fileList = files.map((f) => `- ${f.path}`).join("\n"); + const langSection = buildLanguageSection(languages); + + return `Review the following recently changed files for code quality issues. Read each file, then report findings. + +## Format + +For each finding, state: +- **Severity**: CRITICAL, HIGH, MEDIUM, or INFO +- **File** and **line number** +- **Category** (security, error-handling, type-safety, naming, etc.) +- **Issue** and **suggestion** + +Focus areas:${langSection} + +## Files to Review + +${fileList} + +Read each file, identify issues, and report your findings. Fix any CRITICAL or HIGH issues immediately.`; +} diff --git a/packages/pi-code-review/src/types.ts b/packages/pi-code-review/src/types.ts new file mode 100644 index 0000000..4d985b3 --- /dev/null +++ b/packages/pi-code-review/src/types.ts @@ -0,0 +1,25 @@ +export type ReviewSeverity = "CRITICAL" | "HIGH" | "MEDIUM" | "INFO"; + +export interface ReviewFinding { + readonly severity: ReviewSeverity; + readonly file: string; + readonly line?: number; + readonly category: string; + readonly message: string; + readonly suggestion?: string; +} + +export interface EditedFile { + readonly path: string; + readonly language: string | null; +} + +export interface TurnEdits { + readonly files: readonly EditedFile[]; + readonly turnIndex: number; +} + +export interface ReviewResult { + readonly findings: readonly ReviewFinding[]; + readonly filesReviewed: readonly string[]; +} diff --git a/packages/pi-code-review/tsconfig.build.json b/packages/pi-code-review/tsconfig.build.json new file mode 100644 index 0000000..af8cdbb --- /dev/null +++ b/packages/pi-code-review/tsconfig.build.json @@ -0,0 +1,4 @@ +{ + "extends": "./tsconfig.json", + "exclude": ["node_modules", "dist", "src/**/*.test.ts"] +} diff --git a/packages/pi-code-review/tsconfig.json b/packages/pi-code-review/tsconfig.json new file mode 100644 index 0000000..b1f1a4c --- /dev/null +++ b/packages/pi-code-review/tsconfig.json @@ -0,0 +1,10 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "composite": true, + "outDir": "dist", + "rootDir": "src" + }, + "include": ["src/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/pi-code-review/vitest.config.ts b/packages/pi-code-review/vitest.config.ts new file mode 100644 index 0000000..1c93b60 --- /dev/null +++ b/packages/pi-code-review/vitest.config.ts @@ -0,0 +1,21 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + globals: true, + environment: "node", + include: ["src/**/*.test.ts"], + coverage: { + provider: "v8", + reporter: ["text", "json", "html"], + thresholds: { + global: { + branches: 80, + functions: 80, + lines: 80, + statements: 80, + }, + }, + }, + }, +}); diff --git a/release-please-config.json b/release-please-config.json index b4a6338..6d0922f 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -15,6 +15,7 @@ "packages/pi-continuous-learning": {}, "packages/pi-red-green": {}, "packages/pi-compass": {}, - "packages/pi-simplify": {} + "packages/pi-simplify": {}, + "packages/pi-code-review": {} } } diff --git a/tsconfig.json b/tsconfig.json index 25fa89f..4c1b390 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -5,6 +5,7 @@ { "path": "packages/pi-continuous-learning" }, { "path": "packages/pi-red-green" }, { "path": "packages/pi-compass" }, - { "path": "packages/pi-simplify" } + { "path": "packages/pi-simplify" }, + { "path": "packages/pi-code-review" } ] }