From 555fe67b1718dc7abfefde01c0a6c64f72ba6047 Mon Sep 17 00:00:00 2001 From: amonkhouse Date: Thu, 6 Aug 2026 16:26:31 +0100 Subject: [PATCH 1/3] feat: allow users to add their own review style prompts --- .github/workflows/claude-review.yml | 2 + docs/actions.md | 36 +++++ review-styles/README.md | 54 +++++++ review-styles/amonkhouse.md | 5 + scripts/build-review-prompt.test.ts | 224 ++++++++++++++++++++++++++++ scripts/build-review-prompt.ts | 160 ++++++++++++++++++-- 6 files changed, 470 insertions(+), 11 deletions(-) create mode 100644 review-styles/README.md create mode 100644 review-styles/amonkhouse.md 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..8ab69f0 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -304,6 +304,42 @@ 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`. + +Add `review-styles/.md` (lowercase) to the `duchamp` repo. It's +picked up automatically for PRs you author, based on `github.event.pull_request.user.login`. + +```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. +``` + +`mode` (optional, defaults to `augment`) controls how the file merges with the +default prompt: + +- `augment` (default): keeps the full default prompt (guardrails, review format) + and appends your content as a "Reviewer Style Preferences" section that takes + precedence on tone. +- `replace_style`: keeps the default guardrails and review format, but swaps out + the "How to write" tone guidance for your content. +- `override`: your content becomes the entire prompt - no guardrails or format + are kept unless you write them yourself. + +Precedence: a repo's `.claude-review.yml` `prompt:` field always wins over a +personal style (repo-wide override beats individual preference). Otherwise, a +repo's `focus_areas`/`context`/`ignore_paths` still apply under `augment` and +`replace_style` (scope stays with the repo, tone with the individual) but not +under `override`. + +See [`review-styles/README.md`](../review-styles/README.md) for 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..547f84e 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,104 @@ export const loadRepoConfig = (): RepoConfig | null => { } } -export const buildPrompt = (): string => { - const repoConfig = loadRepoConfig() +/** + * Parse a personal style file's contents into a mode + body. + * + * The file may start with YAML frontmatter (delimited by `---` lines) specifying + * `mode`. Everything after the frontmatter (or the whole file, if there is none) + * is the style content. Pure function - no file IO - so it's easy to unit test. + */ +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"` + ) + } + } + + return { login, mode, content: content.trim() } +} + +/** + * Load the PR author's personal review style, if they have one. + * + * Resolved relative to this script's location (not process.cwd(), which is the + * checked-out PR repo) since style files live centrally in this repo. + */ +export const loadPersonalStyle = ( + author: string | undefined +): PersonalStyle | null => { + if (!author) { + return null } - // Otherwise, build from default + customizations - const sections = [DEFAULT_PROMPT] + 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. Pure function - + * both inputs are already loaded - so merge behavior is directly testable. + */ +export const assemblePrompt = ( + repoConfig: RepoConfig | null, + personalStyle: PersonalStyle | null +): string => { + // A personal full override takes the place of the default prompt entirely, + // but repo-level customization (context/focus areas/ignore paths) still applies - + // see buildPrompt() for the repo `prompt:` override, which beats everything. + 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 +270,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() From 0fa80b715e076345ae14c6ba1bfb927192cad995 Mon Sep 17 00:00:00 2001 From: amonkhouse Date: Thu, 6 Aug 2026 17:05:29 +0100 Subject: [PATCH 2/3] docs: remove duplicate docs --- docs/actions.md | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/docs/actions.md b/docs/actions.md index 8ab69f0..f591b14 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -308,37 +308,7 @@ prompt: | 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`. - -Add `review-styles/.md` (lowercase) to the `duchamp` repo. It's -picked up automatically for PRs you author, based on `github.event.pull_request.user.login`. - -```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. -``` - -`mode` (optional, defaults to `augment`) controls how the file merges with the -default prompt: - -- `augment` (default): keeps the full default prompt (guardrails, review format) - and appends your content as a "Reviewer Style Preferences" section that takes - precedence on tone. -- `replace_style`: keeps the default guardrails and review format, but swaps out - the "How to write" tone guidance for your content. -- `override`: your content becomes the entire prompt - no guardrails or format - are kept unless you write them yourself. - -Precedence: a repo's `.claude-review.yml` `prompt:` field always wins over a -personal style (repo-wide override beats individual preference). Otherwise, a -repo's `focus_areas`/`context`/`ignore_paths` still apply under `augment` and -`replace_style` (scope stays with the repo, tone with the individual) but not -under `override`. - -See [`review-styles/README.md`](../review-styles/README.md) for details. +See `review-styles/README.md` for setup and details. **Security Notes:** From e046bfe5df1844777b815110499e1525046d6284 Mon Sep 17 00:00:00 2001 From: amonkhouse Date: Thu, 6 Aug 2026 17:12:37 +0100 Subject: [PATCH 3/3] docs: reduce wordy comments --- scripts/build-review-prompt.ts | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/scripts/build-review-prompt.ts b/scripts/build-review-prompt.ts index 547f84e..e35e6c1 100644 --- a/scripts/build-review-prompt.ts +++ b/scripts/build-review-prompt.ts @@ -144,13 +144,7 @@ export const loadRepoConfig = (): RepoConfig | null => { } } -/** - * Parse a personal style file's contents into a mode + body. - * - * The file may start with YAML frontmatter (delimited by `---` lines) specifying - * `mode`. Everything after the frontmatter (or the whole file, if there is none) - * is the style content. Pure function - no file IO - so it's easy to unit test. - */ +// Parse a personal style file's contents into a mode + body. export const parsePersonalStyle = ( login: string, raw: string @@ -187,12 +181,8 @@ export const parsePersonalStyle = ( return { login, mode, content: content.trim() } } -/** - * Load the PR author's personal review style, if they have one. - * - * Resolved relative to this script's location (not process.cwd(), which is the - * checked-out PR repo) since style files live centrally in this repo. - */ +// Load the PR author's personal review style, if they have one. + export const loadPersonalStyle = ( author: string | undefined ): PersonalStyle | null => { @@ -219,17 +209,11 @@ export const loadPersonalStyle = ( } } -/** - * Assemble the final prompt from repo config and personal style. Pure function - - * both inputs are already loaded - so merge behavior is directly testable. - */ +// Assemble the final prompt from repo config and personal style. export const assemblePrompt = ( repoConfig: RepoConfig | null, personalStyle: PersonalStyle | null ): string => { - // A personal full override takes the place of the default prompt entirely, - // but repo-level customization (context/focus areas/ignore paths) still applies - - // see buildPrompt() for the repo `prompt:` override, which beats everything. if (personalStyle?.mode === "override") { return personalStyle.content }