diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 2b07ac7..89a0af6 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -196,6 +196,8 @@ jobs: - name: Build review prompt id: prompt run: npx ts-node .tooling/scripts/build-review-prompt.ts + env: + PR_AUTHOR: ${{ github.event.pull_request.user.login }} - name: Cleanup previous AI review comments uses: actions/github-script@v7 diff --git a/docs/actions.md b/docs/actions.md index 7dcba24..f591b14 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -304,6 +304,12 @@ prompt: | Ignore style and formatting issues entirely. ``` +**Personal Review Styles:** + +Individuals can customize the *tone* of reviews on their own PRs, across every +repo that uses this workflow, without touching any repo's `.claude-review.yml`. +See `review-styles/README.md` for setup and details. + **Security Notes:** - Requires approval for external contributors to prevent prompt injection diff --git a/review-styles/README.md b/review-styles/README.md new file mode 100644 index 0000000..145f39a --- /dev/null +++ b/review-styles/README.md @@ -0,0 +1,54 @@ +# Personal review styles + +The [Claude AI PR review](../docs/actions.md#run-claude-reviewyml) uses one default +prompt for everyone. If you want the review of *your* PRs to have a different tone +or emphasis, regardless of which repo the PR is in, add a file here. + +## Setup + +Create `review-styles/.md` (lowercase) in this repo, e.g. +`review-styles/amonkhouse.md`. It's picked up automatically the next time a PR you +author is reviewed, in any repo that uses this workflow. No other setup needed. + +## Format + +Optional YAML frontmatter, then your style content in plain prose or bullets: + +```markdown +--- +mode: augment +--- +Be blunt and terse. Skip the summary on small PRs. +I care most about data-pipeline correctness and idempotency; flag anything +that could double-write or silently drop rows. UK English. +``` + +If you omit the frontmatter, `mode` defaults to `augment`. + +## Modes + +- **`augment`** (default) - keeps the full default prompt (guardrails, review + format, priority emojis) and appends your content as a "Reviewer Style + Preferences" section, with a note that it wins on tone conflicts. Safest option; + right for most people who just want a different voice or emphasis. +- **`replace_style`** - keeps the default guardrails and review format, but swaps + out the "How to write" tone guidance for your content. Use this if you have + strong, complete opinions on tone and don't want the default tone advice + competing with yours. +- **`override`** - your content becomes the *entire* prompt. Nothing else is kept: + no false-positive guardrails, no required review format, no posting + instructions, unless you write them yourself. Only use this if you're + deliberately writing a full replacement prompt (mirrors the repo-level `prompt:` + field in `.claude-review.yml`, but scoped to you). + +## Precedence + +1. A repo's `.claude-review.yml` `prompt:` field (a full override for that repo) + always wins over your personal style - it's a deliberate repo-wide decision. +2. Otherwise, your personal style is applied per its `mode`. +3. A repo's `focus_areas`, `context`, and `ignore_paths` still apply in `augment` + and `replace_style` modes - scope stays with the repo, tone stays with you. + They do not apply under `override`, since there is no base prompt left to + append them to. + +See `amonkhouse.md` in this directory for a worked example. diff --git a/review-styles/amonkhouse.md b/review-styles/amonkhouse.md new file mode 100644 index 0000000..16d0042 --- /dev/null +++ b/review-styles/amonkhouse.md @@ -0,0 +1,5 @@ +--- +mode: augment +--- +UK English. Be direct: state the issue and the fix, then stop, no closing summary. +I love emojis. Use more of them. Especially the frog. diff --git a/scripts/build-review-prompt.test.ts b/scripts/build-review-prompt.test.ts index 3358f17..099efa7 100644 --- a/scripts/build-review-prompt.test.ts +++ b/scripts/build-review-prompt.test.ts @@ -1,8 +1,14 @@ import * as fs from "fs" import { + assemblePrompt, buildPrompt, DEFAULT_PROMPT, + HOW_TO_WRITE, + loadPersonalStyle, loadRepoConfig, + PROMPT_CLOSING, + PROMPT_HEADER, + parsePersonalStyle, } from "./build-review-prompt" jest.mock("fs") @@ -104,6 +110,7 @@ prompt: | describe("buildPrompt", () => { beforeEach(() => { jest.clearAllMocks() + delete process.env.PR_AUTHOR }) it("returns default prompt when no config exists", () => { @@ -178,6 +185,223 @@ ignore_paths: expect(result).toContain("## Files to Skip") expect(result).toContain("- **/*.generated.ts") }) + + it("applies the PR author's personal style when PR_AUTHOR is set", () => { + process.env.PR_AUTHOR = "Amonkhouse" + mockFs.existsSync.mockImplementation( + p => typeof p === "string" && p.endsWith("amonkhouse.md") + ) + mockFs.readFileSync.mockImplementation(p => { + if (typeof p === "string" && p.endsWith("amonkhouse.md")) { + return "Be blunt and terse." + } + throw new Error(`unexpected read: ${String(p)}`) + }) + + const result = buildPrompt() + + expect(result).toContain("## Reviewer Style Preferences (amonkhouse)") + expect(result).toContain("Be blunt and terse.") + expect(result).toContain("### Summary") // default prompt structure retained + }) + + it("ignores personal style when no PR_AUTHOR is set", () => { + mockFs.existsSync.mockReturnValue(false) + + const result = buildPrompt() + + expect(result).not.toContain("Reviewer Style Preferences") + expect(result).toBe(DEFAULT_PROMPT) + }) + + it("repo-level prompt override still beats a personal style", () => { + process.env.PR_AUTHOR = "amonkhouse" + mockFs.existsSync.mockReturnValue(true) + mockFs.readFileSync.mockImplementation(p => { + if (typeof p === "string" && p.endsWith(".claude-review.yml")) { + return ` +prompt: | + You are a custom security reviewer. +` + } + return "Be blunt and terse." + }) + + const result = buildPrompt() + + expect(result).toContain("You are a custom security reviewer.") + expect(result).not.toContain("Reviewer Style Preferences") + }) +}) + +describe("parsePersonalStyle", () => { + it("defaults to augment mode with no frontmatter", () => { + const result = parsePersonalStyle("amonkhouse", "Be blunt and terse.") + + expect(result).toEqual({ + login: "amonkhouse", + mode: "augment", + content: "Be blunt and terse.", + }) + }) + + it("reads mode from frontmatter", () => { + const result = parsePersonalStyle( + "amonkhouse", + "---\nmode: replace_style\n---\nBe blunt and terse.\n" + ) + + expect(result.mode).toBe("replace_style") + expect(result.content).toBe("Be blunt and terse.") + }) + + it("falls back to augment and warns on an unknown mode", () => { + const consoleSpy = jest.spyOn(console, "error").mockImplementation() + + const result = parsePersonalStyle( + "amonkhouse", + "---\nmode: nonsense\n---\nBe blunt and terse.\n" + ) + + expect(result.mode).toBe("augment") + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("Unknown mode") + ) + consoleSpy.mockRestore() + }) + + it("falls back to augment and warns on malformed frontmatter", () => { + const consoleSpy = jest.spyOn(console, "error").mockImplementation() + + const result = parsePersonalStyle( + "amonkhouse", + "---\nmode: [unterminated\n---\nBe blunt and terse.\n" + ) + + expect(result.mode).toBe("augment") + expect(consoleSpy).toHaveBeenCalled() + consoleSpy.mockRestore() + }) + + it("supports override mode", () => { + const result = parsePersonalStyle( + "amonkhouse", + "---\nmode: override\n---\nYou are a custom reviewer.\n" + ) + + expect(result.mode).toBe("override") + expect(result.content).toBe("You are a custom reviewer.") + }) +}) + +describe("loadPersonalStyle", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it("returns null when no author is given", () => { + expect(loadPersonalStyle(undefined)).toBeNull() + }) + + it("returns null when no style file exists for the author", () => { + mockFs.existsSync.mockReturnValue(false) + + expect(loadPersonalStyle("amonkhouse")).toBeNull() + }) + + it("lowercases the login when resolving the file path", () => { + mockFs.existsSync.mockImplementation( + p => typeof p === "string" && p.endsWith("review-styles/amonkhouse.md") + ) + mockFs.readFileSync.mockReturnValue("Be blunt and terse.") + + const result = loadPersonalStyle("AmonKHouse") + + expect(result?.login).toBe("amonkhouse") + expect(result?.content).toBe("Be blunt and terse.") + }) + + it("returns null and logs a warning on read error", () => { + const consoleSpy = jest.spyOn(console, "error").mockImplementation() + mockFs.existsSync.mockReturnValue(true) + mockFs.readFileSync.mockImplementation(() => { + throw new Error("Read error") + }) + + const result = loadPersonalStyle("amonkhouse") + + expect(result).toBeNull() + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("Failed to read review-styles/amonkhouse.md") + ) + consoleSpy.mockRestore() + }) +}) + +describe("assemblePrompt", () => { + it("returns the default prompt when there is no repo config or personal style", () => { + expect(assemblePrompt(null, null)).toBe(DEFAULT_PROMPT) + }) + + it("augment mode keeps the default prompt and appends a style section", () => { + const result = assemblePrompt(null, { + login: "amonkhouse", + mode: "augment", + content: "Be blunt and terse.", + }) + + expect(result).toContain(DEFAULT_PROMPT) + expect(result).toContain("## Reviewer Style Preferences (amonkhouse)") + expect(result).toContain("Be blunt and terse.") + expect(result).toContain("prefer these") + }) + + it("replace_style mode keeps guardrails and format but swaps the tone block", () => { + const result = assemblePrompt(null, { + login: "amonkhouse", + mode: "replace_style", + content: "Be blunt and terse.", + }) + + expect(result).toContain(PROMPT_HEADER) + expect(result).toContain(PROMPT_CLOSING) + expect(result).toContain("## How to write\nBe blunt and terse.") + expect(result).not.toContain(HOW_TO_WRITE) + expect(result).not.toContain("Reviewer Style Preferences") + }) + + it("override mode returns only the personal content", () => { + const result = assemblePrompt( + { focus_areas: ["Watch for N+1 queries"] }, + { + login: "amonkhouse", + mode: "override", + content: "You are a custom reviewer.", + } + ) + + expect(result).toBe("You are a custom reviewer.") + }) + + it("augment mode still applies repo focus areas and ignore paths", () => { + const result = assemblePrompt( + { + focus_areas: ["Watch for N+1 queries"], + ignore_paths: ["**/*.generated.ts"], + }, + { login: "amonkhouse", mode: "augment", content: "Be blunt and terse." } + ) + + expect(result).toContain("## Additional Focus Areas") + expect(result).toContain("- Watch for N+1 queries") + expect(result).toContain("## Files to Skip") + expect(result).toContain("- **/*.generated.ts") + expect(result).toContain("## Reviewer Style Preferences (amonkhouse)") + // Style preferences should come after repo customizations + expect(result.indexOf("## Files to Skip")).toBeLessThan( + result.indexOf("## Reviewer Style Preferences") + ) + }) }) describe("DEFAULT_PROMPT", () => { diff --git a/scripts/build-review-prompt.ts b/scripts/build-review-prompt.ts index 428e767..e35e6c1 100644 --- a/scripts/build-review-prompt.ts +++ b/scripts/build-review-prompt.ts @@ -3,13 +3,21 @@ import * as yaml from "js-yaml" import * as path from "path" /** - * Build a review prompt by merging default Artsy guidelines with repo-specific configuration. + * Build a review prompt by merging default Artsy guidelines with repo-specific + * configuration and the PR author's personal review style. * * Repos can create a .claude-review.yml file with: * - prompt: Complete custom prompt (overrides everything else) * - focus_areas: Array of specific things to watch for (added to default prompt) * - ignore_paths: Glob patterns for files to skip * - context: Additional context about the codebase + * + * Individuals can create a review-styles/.md file (in this repo) with + * optional frontmatter choosing how it merges with the default prompt: + * - mode: augment (default) - keep the default prompt, append the style as a + * preference section + * - mode: replace_style - keep guardrails/format, swap only the "How to write" tone + * - mode: override - the personal file becomes the entire prompt */ interface ExcludeConfig { @@ -25,7 +33,21 @@ interface RepoConfig { exclude?: ExcludeConfig } -export const DEFAULT_PROMPT = `You are a senior staff engineer conducting a code review. +export type PersonalStyleMode = "augment" | "replace_style" | "override" + +const VALID_PERSONAL_STYLE_MODES: PersonalStyleMode[] = [ + "augment", + "replace_style", + "override", +] + +export interface PersonalStyle { + login: string + mode: PersonalStyleMode + content: string +} + +export const PROMPT_HEADER = `You are a senior staff engineer conducting a code review. You have access to the full codebase. The PR branch has been checked out. ## Critical: Avoid False Positives @@ -83,7 +105,9 @@ Briefly note any concerns in these areas (skip if nothing notable): ### Questions for Author List anything unclear that needs clarification before you can fully assess the PR. -## How to write +` + +export const HOW_TO_WRITE = `## How to write - Lead with the problem. No preamble like "I noticed that" or "It might be worth considering". - Short words, active voice: "this leaks the handle", not "a resource leak may be introduced". - Cut every word that adds nothing. "Because", not "due to the fact that"; "to", not "in order to"; "before", not "prior to". @@ -93,12 +117,16 @@ List anything unclear that needs clarification before you can fully assess the P - Avoid words like leverage, robust, comprehensive, crucial, seamless, delve, streamline. Use everyday words. - Go easy on em-dashes; prefer commas and full stops. ---- +` + +export const PROMPT_CLOSING = `--- Be constructive and explain your reasoning. Focus on substantive issues, not style nitpicks. Remember: An empty "Issues Found" section is a valid and often correct outcome. The goal is accurate review, not comprehensive critique. ` +export const DEFAULT_PROMPT = PROMPT_HEADER + HOW_TO_WRITE + PROMPT_CLOSING + export const loadRepoConfig = (): RepoConfig | null => { const configPath = path.join(process.cwd(), ".claude-review.yml") @@ -116,16 +144,88 @@ export const loadRepoConfig = (): RepoConfig | null => { } } -export const buildPrompt = (): string => { - const repoConfig = loadRepoConfig() +// Parse a personal style file's contents into a mode + body. +export const parsePersonalStyle = ( + login: string, + raw: string +): PersonalStyle => { + const frontmatterMatch = raw.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n?/) + + let rawMode: unknown + let content = raw + + if (frontmatterMatch) { + content = raw.slice(frontmatterMatch[0].length) + try { + const frontmatter = yaml.load(frontmatterMatch[1]) as { mode?: unknown } + rawMode = frontmatter?.mode + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error( + `Warning: Failed to parse frontmatter in review-styles/${login}.md: ${message}` + ) + } + } - // If repo provides a complete custom prompt, use it directly - if (repoConfig?.prompt) { - return repoConfig.prompt + let mode: PersonalStyleMode = "augment" + if (rawMode !== undefined) { + if (VALID_PERSONAL_STYLE_MODES.includes(rawMode as PersonalStyleMode)) { + mode = rawMode as PersonalStyleMode + } else { + console.error( + `Warning: Unknown mode "${String(rawMode)}" in review-styles/${login}.md, falling back to "augment"` + ) + } } - // Otherwise, build from default + customizations - const sections = [DEFAULT_PROMPT] + return { login, mode, content: content.trim() } +} + +// Load the PR author's personal review style, if they have one. + +export const loadPersonalStyle = ( + author: string | undefined +): PersonalStyle | null => { + if (!author) { + return null + } + + const login = author.toLowerCase() + const stylePath = path.join(__dirname, "..", "review-styles", `${login}.md`) + + if (!fs.existsSync(stylePath)) { + return null + } + + try { + const raw = fs.readFileSync(stylePath, "utf8") + return parsePersonalStyle(login, raw) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error( + `Warning: Failed to read review-styles/${login}.md: ${message}` + ) + return null + } +} + +// Assemble the final prompt from repo config and personal style. +export const assemblePrompt = ( + repoConfig: RepoConfig | null, + personalStyle: PersonalStyle | null +): string => { + if (personalStyle?.mode === "override") { + return personalStyle.content + } + + const base = + personalStyle?.mode === "replace_style" + ? PROMPT_HEADER + + `## How to write\n${personalStyle.content}\n\n` + + PROMPT_CLOSING + : DEFAULT_PROMPT + + const sections = [base] if (repoConfig) { // Add repo-specific context @@ -154,9 +254,31 @@ export const buildPrompt = (): string => { } } + if (personalStyle?.mode === "augment") { + sections.push( + `\n## Reviewer Style Preferences (${personalStyle.login})\n\n` + + `The PR author has personal review-style preferences below. Where these conflict with the guidance above, prefer these.\n\n` + + `${personalStyle.content}\n` + ) + } + return sections.join("") } +export const buildPrompt = (): string => { + const repoConfig = loadRepoConfig() + + // If repo provides a complete custom prompt, use it directly - this is a + // deliberate repo-wide decision and beats any personal style. + if (repoConfig?.prompt) { + return repoConfig.prompt + } + + const personalStyle = loadPersonalStyle(process.env.PR_AUTHOR) + + return assemblePrompt(repoConfig, personalStyle) +} + const main = (): void => { const prompt = buildPrompt()