Skip to content

feat: allow users to add their own review style prompts - #113

Merged
amonkhouse merged 3 commits into
mainfrom
add-individual-review-styles
Aug 7, 2026
Merged

feat: allow users to add their own review style prompts#113
amonkhouse merged 3 commits into
mainfrom
add-individual-review-styles

Conversation

@amonkhouse

@amonkhouse amonkhouse commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Adds support for individual, per-author review styles in the Claude PR review workflow, on top of the existing per-repo .claude-review.yml customisation.

Previously the review prompt was a single hardcoded default merged only with per-repo config. Different engineers want different review tone and emphasis regardless of which repo they're working in. This adds a lightweight, centrally-managed mechanism for that: a personal file that's picked up automatically based on the PR author's GitHub login name, with no changes needed in any consuming repos.

Note however that repo-specific review prompts will currently cause user-specific settings to be ignored. We can come back and redesign that if this is a feature we like.

  • Added support for a review-styles/<github-login>.md file per person, loaded via the PR author's login and merged into the review prompt.
  • Personal style files can declare a mode via YAML frontmatter (augment, replace_style, or override), controlling how much of the default prompt (guardrails, review format) their style keeps versus replaces.
  • Refactored DEFAULT_PROMPT in build-review-prompt.ts into named sections (PROMPT_HEADER, HOW_TO_WRITE, PROMPT_CLOSING) so the tone section can be swapped independently.
  • Threaded the PR author's login into the workflow via a new PR_AUTHOR env var on the prompt-build step.
  • Repo-level prompt: overrides in .claude-review.yml still take precedence over personal styles.
  • Added a review-styles/README.md explaining setup and the three modes, an example personal style file, and documentation in docs/actions.md.
  • Added test coverage for frontmatter parsing, mode fallback behaviour, all three merge modes, and precedence against repo-level config.

Co-authored with Claude Opus 5 (plan) and Sonnet 5 (code).

Comment thread scripts/build-review-prompt.ts Outdated

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

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.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Detected new or modified files in .github/workflows/. Please ensure you update the relevant documentation alongside these workflow changes.

@github-actions github-actions Bot deleted a comment from claude Bot Aug 7, 2026
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` +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This note is broader than the docs describe, and it sits after the repo's focus_areas/context/ignore_paths sections, so "the guidance above" includes them.

But "Where these conflict with the guidance above, prefer these" tells the model to prefer the personal style over everything above, including the repo's focus areas and the "Avoid False Positives" guardrails. That contradicts the documented precedence and weakens the "safest option, keeps guardrails" framing.

Scope the instruction to tone/formatting:

Suggested change
`The PR author has personal review-style preferences below. Where these conflict with the guidance above, prefer these.\n\n` +
`The PR author has personal review-style preferences below. Where these conflict with the *tone or formatting* guidance above, prefer these. They do not override the review scope, guardrails, or repo focus areas.\n\n` +

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds per-author "personal review styles": a PR author can drop review-styles/<login>.md in the tooling repo to change the tone or emphasis of reviews on their own PRs across any repo using this workflow. Three merge modes (augment, replace_style, override), a new PR_AUTHOR env var passed from the workflow, docs, and thorough unit tests.

The trust boundary is right: styles are read from the artsy/duchamp@main checkout (__dirname/../review-styles), not the PR under review, so an author can't inject a style through their own PR content, and PR_AUTHOR comes from github.event.pull_request.user.login (not attacker-controlled text). Path resolution and the login lowercasing are handled correctly, and GitHub logins can't contain ../, so there's no traversal risk.

Issues Found

🟡 Important — augment precedence note is broader than documented (scripts/build-review-prompt.ts:260)

The appended note says "Where these conflict with the guidance above, prefer these." That section is pushed after the repo's focus_areas/context/ignore_paths, so "the guidance above" covers them and the "Avoid False Positives" guardrails. This contradicts the README:

As written, a personal style can instruct the model to override repo focus areas or the guardrails, undermining the "safest option, keeps guardrails" framing. Suggested wording fix left inline.

🟢 Suggestion — override docs overstate what's dropped (review-styles/README.md:38-40)

The bullet says override keeps "no posting instructions, unless you write them yourself." But the workflow always appends the ## How to Post Your Review block and the REPO/PR header after steps.prompt.outputs.review_prompt, so posting instructions survive override (same as the repo-level prompt: override). Minor, but worth a one-line clarification so authors don't re-add them redundantly.

Areas Reviewed

  • Architecture & Design: Clean split — parsePersonalStyle / loadPersonalStyle / assemblePrompt are pure and separately tested. Repo config (from target repo cwd) vs personal style (from tooling __dirname) resolve from the correct sources.
  • Security: Trust boundary is sound (see summary). No traversal via login. override removing guardrails is gated behind a reviewed commit to duchamp, so acceptable.
  • Bugs & Edge Cases: Frontmatter parsing handles missing/malformed/unknown-mode cases with warnings and an augment fallback; all covered by tests.
  • Testing: Good coverage across parse/load/assemble and the buildPrompt integration paths, including repo-override-beats-personal and case-insensitive login.

Questions for Author

  • In augment/replace_style, when both a repo focus_areas and a personal style are present, which is intended to win on a genuine conflict? The README (repo scope wins) and the code (prefer these) currently disagree — confirming intent will settle whether the inline wording fix is the right direction.

@MounirDhahri MounirDhahri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! very welcome addition

@MounirDhahri

Copy link
Copy Markdown
Member

Also, it's kind of a mindset shift, from AI being an external that reviews your work as a colleague, who might louder, nitpicking (except that it makes more mistakes), to being a pairing buddy or something like that.

@amonkhouse
amonkhouse merged commit 19b759d into main Aug 7, 2026
10 of 17 checks passed
amonkhouse added a commit that referenced this pull request Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants