Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/claude-review.yml

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a discussion to be had here where we should keep these files, if this is a change we want to keep. I personally feel they should be in their own repo, or even s3, so that people can freely make changes to their own files.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions docs/actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions review-styles/README.md
Original file line number Diff line number Diff line change
@@ -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/<your-github-login>.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.
5 changes: 5 additions & 0 deletions review-styles/amonkhouse.md

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add a more serious one but just wanted to test this out aha

Original file line number Diff line number Diff line change
@@ -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.
224 changes: 224 additions & 0 deletions scripts/build-review-prompt.test.ts
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -104,6 +110,7 @@ prompt: |
describe("buildPrompt", () => {
beforeEach(() => {
jest.clearAllMocks()
delete process.env.PR_AUTHOR
})

it("returns default prompt when no config exists", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
Loading
Loading