diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index 40d58ebbc..c9c23daf0 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -1,18 +1,18 @@ --- name: review-pr description: > - Review a branch diff with dual agents before merge. Use when the user says - "review-pr", "review this PR", "review this branch", or invokes /review-pr. - Spawns Hoshe (code quality) and Tyre (architecture) in parallel to review - the diff against main. Reports approve/reject with inline comments. + Review a branch diff with team-appropriate agents before merge. Use when the + user says "review-pr", "review this PR", "review this branch", or invokes + /review-pr. Spawns reviewers matched to the branch type (code, copy, visual, + audio) in parallel. Reports approve/reject with inline comments. user-invocable: true allowed-tools: Bash, Read, Grep, Glob, Task --- # PR Review Skill -Dual-agent review of a branch diff against main. Hoshe reviews code quality, -Tyre reviews architecture. Both must approve for a clean review. +Multi-agent review of a branch diff against main. Reviewer composition depends +on the branch type. All reviewers must approve for a clean review. ## Workflow @@ -32,7 +32,21 @@ Fetch remote branches first: git fetch --all ``` -### 2. Generate the diff +### 2. Determine reviewer team + +Map the branch name to a reviewer set. Use the branch prefix (before any `/` +or `-` suffix) to classify: + +| Branch type | Branches | Reviewers | +|-------------|----------|-----------| +| **code** | `server`, `client`, `ci`, or unknown | Hoshe (code quality) + Tyre (architecture) | +| **copy** | `copy` | Hoshe (QA) + Paula (narrative depth) + Miri (world consistency) | +| **visual** | `visual` | Hoshe (QA) + Araminta (art direction) | +| **audio** | `audio` | Hoshe (QA) + Ozzie (player experience) | + +If the branch name doesn't match any known type, default to **code** reviewers. + +### 3. Generate the diff ```bash git log --oneline main.. @@ -56,14 +70,17 @@ For large diffs (>1000 lines of source), provide **source files** rather than raw diff to reviewers — cleaner context, better reviews. Read files with `git show origin/:` and include them in the prompt. -### 3. Spawn both reviewers in parallel +### 4. Spawn reviewers in parallel -Use the Task tool to spawn **two agents simultaneously** in a single message. -Use `model: sonnet` for both — sufficient for review, saves cost. +Use the Task tool to spawn **all reviewers simultaneously** in a single message. +Use `model: sonnet` for all — sufficient for review, saves cost. -**Agent 1 — Hoshe (Code Quality)** -- `subagent_type`: `hoshe` -- `model`: `sonnet` +--- + +#### Code reviews (`server`, `client`, `ci`) + +**Hoshe (Code Quality)** +- `subagent_type`: `hoshe`, `model`: `sonnet` - Prompt: Include source code and commit log. Ask Hoshe to review for: - Correctness and bug risks - Error handling gaps @@ -71,11 +88,9 @@ Use `model: sonnet` for both — sufficient for review, saves cost. - Code style and clarity - Security concerns (OWASP top 10, injection risks) - Performance issues -- Request structured verdict: APPROVE or REQUEST_CHANGES with file-specific comments -**Agent 2 — Tyre (Architecture)** -- `subagent_type`: `tyre` -- `model`: `sonnet` +**Tyre (Architecture)** +- `subagent_type`: `tyre`, `model`: `sonnet` - Prompt: Include source code and commit log. Tell Tyre to read the relevant `decisions/*.md` files first, then review for: - Architectural consistency with project decisions @@ -84,40 +99,119 @@ Use `model: sonnet` for both — sufficient for review, saves cost. - Scalability implications - Whether the change respects non-negotiable baselines (D-010, D-012) - Tyre can read files directly from the branch using `git show origin/:` -- Request structured verdict: APPROVE or REQUEST_CHANGES with file-specific comments -### 4. Present results +--- -Format the combined review as a table per reviewer: +#### Copy reviews (`copy`) + +**Hoshe (QA)** +- `subagent_type`: `hoshe`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Ask Hoshe to review for: + - Formatting consistency (markdown, file naming, frontmatter) + - Broken references or links + - Spelling and grammar + - File organization and structure + - Missing or orphaned files + +**Paula (Narrative Depth)** +- `subagent_type`: `paula`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Tell Paula to read the + relevant `decisions/*.md` files first, then review for: + - Narrative quality and character voice consistency + - Whether dialogue and monologue feel authentic to the characters + - Consequences and stakes — do choices carry weight? + - Political and interpersonal depth + - Emotional resonance — does the text make you feel something? + +**Miri (World Consistency)** +- `subagent_type`: `miri`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Tell Miri to read the + relevant `decisions/*.md` files first, then review for: + - Lore accuracy — do facts match established setting? + - Internal consistency across files + - IP originality — nothing should read as a copy from another franchise + - Faction, technology, and location details match the worldbuilding docs + - Setting serves gameplay mechanics (asymmetric information, perception) + +--- + +#### Visual reviews (`visual`) + +**Hoshe (QA)** +- `subagent_type`: `hoshe`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Ask Hoshe to review for: + - File format and naming conventions + - Asset organization and directory structure + - Missing or broken references in scene/resource files + - Import settings consistency + +**Araminta (Art Direction)** +- `subagent_type`: `araminta`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Tell Araminta to read + the style guide and relevant design docs first, then review for: + - Visual consistency with the established style guide + - Color palette adherence + - UI pattern consistency (diegetic-first, clarity over beauty) + - Whether assets scale gracefully (boxes-with-labels to full-art) + - Mood and tone — sleek, advanced, subtle Commonwealth aesthetic + +--- + +#### Audio reviews (`audio`) + +**Hoshe (QA)** +- `subagent_type`: `hoshe`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Ask Hoshe to review for: + - File format and naming conventions + - Audio asset organization and directory structure + - Missing or broken references + - Import/bus configuration consistency + +**Ozzie (Player Experience)** +- `subagent_type`: `ozzie`, `model`: `sonnet` +- Prompt: Include the changed files and commit log. Ask Ozzie to review for: + - Emotional impact — does the audio enhance the moment? + - Atmosphere and tone — does it feel like the Commonwealth? + - Player feedback clarity — can the player tell what just happened? + - Pacing — do sounds support or fight the gameplay rhythm? + - Memorable moments — will players remember these audio cues? + +--- + +All reviewers: request structured verdict: APPROVE or REQUEST_CHANGES with +file-specific comments. + +### 5. Present results + +Format the combined review as a table per reviewer. Include one section per +reviewer that was spawned (2 for code/visual/audio, 3 for copy): ``` -## Review: -> main +## Review: -> main (type: code|copy|visual|audio) -### Hoshe (Code Quality): [APPROVE | REQUEST_CHANGES] +### (): [APPROVE | REQUEST_CHANGES] [Summary] | # | File | Severity | Issue | |---|------|----------|-------| | 1 | path:line | critical/warning/suggestion | description | -### Tyre (Architecture): [APPROVE | REQUEST_CHANGES] -[Summary] -| # | File | Severity | Issue | -|---|------|----------|-------| -| 1 | path:line | critical/warning/suggestion | description | +### (): [APPROVE | REQUEST_CHANGES] +... ### Verdict: [APPROVED | CHANGES REQUESTED] ``` -The overall verdict is APPROVED only if **both** reviewers approve. +The overall verdict is APPROVED only if **all** reviewers approve. ## Prompt template for reviewers Use this structure when constructing the agent prompts (adapt as needed): ``` -Review the following branch diff for merge into main. +Review the following {branch_type} branch diff for merge into main. Branch: {branch} +Branch type: {branch_type} (code|copy|visual|audio) Commits: {commit_log} @@ -126,7 +220,7 @@ Diff stats: [Source files or diff here — exclude vendor/generated code] -Review focus: {focus_area} +Your review focus: {focus_area} Respond with: 1. Verdict: APPROVE or REQUEST_CHANGES @@ -138,7 +232,7 @@ Respond with: If no issues found, say APPROVE with a brief positive summary. ``` -## Posting results to Gitea +## 6. Posting results to Gitea After presenting results to the user, post the review as a PR comment: @@ -149,7 +243,7 @@ tea comment --login schweitz --repo jpmschweitzer/settled-reach " "$(cat <<'REVIEW' -## Dual-Agent Review: -> main +## Review: -> main ...review content... REVIEW )" @@ -163,7 +257,7 @@ Note: `tea pr reject` does not work on your own PRs. Use `tea comment` instead. tea comment --login schweitz --repo jpmschweitzer/settled-reach "$(cat /tmp/review.md)" ``` -## Merging approved PRs +## 7. Merging approved PRs `tea pr merge` fails (405) when branches have conflicts with main. Merge locally instead: