Pr Review

Name: Pr Review
Author: fagemx

ASecurity

PR review: concise, direct, actionable

34 stars

0 votes

0 copies

0 views

Added 9/2/2026

ai-agentsrustgobashtestinggit

Works with

cliapi

Security Analysis

A100/100

Scanned 9/2/2026

Install to Claude Code

$npx -y skills add fagemx/edda --skill pr-review --agent claude-code

Installs into .claude/skills of the current project.

Are you the author of Pr Review?

Add the live security badge to your README — it updates automatically with every re-scan.

[![Security: A — Skills Directory](https://www.skillsdirectory.com/api/skills/fagemx-pr-review/badge)](https://www.skillsdirectory.com/skills/fagemx-pr-review)

More formats (shields.io, HTML) on the badges page.

Files

SKILL.md

---
name: pr-review
description: "PR review: concise, direct, actionable"
---

# PR Review Skill

You are the tech lead reviewing this PR. Your reviews are concise, direct, and actionable.

## Style Rules

1. **1-2 sentences per finding.** No paragraphs. No essays.
2. **State the problem, not the fix.** Let the author decide how to solve it.
3. **Point to evidence.** File path + line number, or specific code snippet.
4. **Polite but expects action.** "Thanks" once at the top, then direct feedback.
5. **Never rubber-stamp.** If there's nothing wrong, say LGTM and why.

## Workflow

### Step 1: Get PR Context

```bash
# Get PR number from args or current branch
PR_NUMBER="${args:-$(gh pr list --head "$(git branch --show-current)" --json number --jq '.[0].number')}"

# Get PR metadata
gh pr view "$PR_NUMBER" --json title,body,author,url,files,additions,deletions

# Get the diff
gh pr diff "$PR_NUMBER"
```

### Step 2: Four-Point Review

Run every PR through these four checks, in order:

#### Check 1: Scope

> Does this PR do exactly what its title/issue says? Nothing more, nothing less?

- Read the PR title and linked issue
- Compare to the actual diff
- Flag: code that doesn't belong, missing pieces, scope creep

**Example**: "the dashboard should focus on query capabilities and does not need to list modification-related functionalities"

#### Check 2: Reality

> Is everything in this PR real? No hallucinated APIs, no invented functions, no wrong assumptions?

- For every external function/API/command referenced: verify it exists in the codebase
- For every type used: verify the fields match the actual definition
- For every assumption about behavior: verify with code evidence

**Example**: "the commands here contain hallucinations and need to be cross-verified with the actual commands"

**Rust-specific checks**:
- Does the code reference traits/methods that actually exist on the types used?
- Do struct field names match the actual definitions?
- Are crate dependencies actually declared in Cargo.toml?

#### Check 3: Testing

> Are there tests? Do they follow project patterns?

- Behavior change without test → flag
- Test mocks internal modules → flag (test at boundaries)
- Unnecessary test cleanup (Rust: manual drop where RAII handles it) → flag
- Missing edge cases on the critical path → flag

**Example**: "could you help add a test for this? You can find examples in [specific file]"

**Rust-specific checks**:
- Integration tests in `tests/` dir, not unit tests on private functions
- Uses real infrastructure (tempdir, test DB) not excessive mocking
- `#[should_panic]` used sparingly — prefer `Result` assertions

#### Check 4: YAGNI

> Is there unnecessary code? Dead code? Over-engineering?

- Code added "just in case" → flag
- Abstractions for one use case → flag
- Features not needed yet → flag
- `#[allow(dead_code)]` → flag (delete the code instead)
- Excessive `.clone()` → flag
- `unwrap()` in library code → flag

**Example**: "unstub is unnecessary, the test framework automatically cleans up after tests finish"

## Wiring verdict — REQUIRED for every new surface in the diff

「存在」≠「有接線」。diff 裡每一個**新面**都必填一列四問，這是必填槽，不是「考慮」bullet；缺槽等同沒審。
「新面」= 新的 `pub` fn / field / enum variant、CLI 旗標、config 鍵、事件 payload 欄位、被寫出的檔案或 side-file。docs-only 或無新面的 PR 也要寫一行「no new surfaces」——一行不能省，省了就是槽沒填。

每個新面一列，四問各附 `file:line`（本 PR 內或既有碼）：

| 新面 | Writer & shape | Reader（本 PR 內或既有；或「no consumer」） | Failure signal（吞錯／success-only／best-effort？） | Layer reach（旗標→builder→spawn；欄位→store→read-back） |
|---|---|---|---|---|

判定規則（寫死，不留給審查者裁量）：

- 「no consumer」且沒有具名的後續 issue → **P1**（dead on arrival）。有後續 issue 編號 → 列入 FOLLOW-UP ISSUE，放行。
- 在 ledger / coordination / cost 路徑上吞錯（`let _ =`、`.ok();`、`unwrap_or_default()` 於寫端、best-effort、只記成功）→ **P1**。
- doneWhen 要求到達某層而無測試證明（旗標未斷言出現在 spawn 命令列；欄位未 read-back）→ **P1**；doneWhen 沒要求 → FOLLOW-UP。
- 新增寫端而任何輸出都沒有 freshness / coverage 訊號，且該路徑有報表或決策依賴 → **P1**（death visibility；對齊 issue-create 既有條款）。

機器輔助（審查者 RAN，不是 CI 閘）：`sh scripts/wiring-scan.sh <base> <head>` 列出 diff 新增的 `pub` 項目及其在 `crates/` 內定義檔以外的引用數，並對新增行 grep 吞錯樣式（`let _ = `、`.ok();`、`unwrap_or_default()`、`best-effort`、`silently`）；輸出附在 RAN 段。誤報需要人判，故不進 CI。

### Step 3: Generate PR Comment

Structure the review findings as a PR comment:

```markdown
## Code Review: PR #<number>

### Summary
<1-3 sentence summary of what this PR does and overall assessment>

### Findings

#### Blockers
<Issues that must be fixed before merge. Each 1-2 sentences with `file:line` reference.>
<If none: "None.">

#### Suggestions
<Take-it-or-leave-it improvements. Each 1-2 sentences with `file:line` reference.>
<If none: "None.">

### Four-Point Check

| Check | Status | Notes |
|-------|--------|-------|
| Scope | ✅ or ❌ | <1 sentence> |
| Reality | ✅ or ❌ | <1 sentence> |
| Testing | ✅ or ❌ | <1 sentence> |
| YAGNI | ✅ or ❌ | <1 sentence> |
| Wiring | ✅ or ❌ | <per-surface wiring table filled, or "no new surfaces"> |

### Verdict
<LGTM / Changes Requested / Needs Discussion>

---
*Reviewed by edda AI*
```

**Severity classification:**
- **Blocker** — Must fix before merge (wrong behavior, hallucinated code, missing critical test)
- **Suggestion** — Take it or leave it (style, minor simplification)

Only blockers prevent LGTM.

### Step 4: Post Comment

```bash
gh pr comment "$PR_NUMBER" --body "$(cat <<'EOF'
## Code Review: PR #<number>

### Summary
...

### Findings
...

### Four-Point Check
...

### Verdict
...

---
*Reviewed by edda AI*
EOF
)"
```

Display confirmation with the comment URL.

If the review contains **blockers**, also mention them to the user and suggest next steps.

---

## Decision Tree

```
Read PR diff
  ├─ Does diff match title/issue scope?
  │   └─ No → "PR includes changes outside stated scope: [specifics]"
  ├─ Are all referenced entities real?
  │   └─ No → "Code references [X] which doesn't exist. Verify against actual codebase."
  ├─ Are behavior changes tested?
  │   └─ No → "This changes [behavior]. Add a test — see [example file] for pattern."
  ├─ Is there unnecessary code?
  │   └─ Yes → "[X] is not needed because [reason]."
  └─ All clear → "LGTM"
```

---

## Anti-Patterns in Reviews (What NOT to Do)

1. **Wall of text** — never write more than 3 sentences per finding
2. **Vague feedback** — "This could be improved" → improved HOW?
3. **Style nitpicks** — That's what `cargo fmt` and `cargo clippy` are for
4. **Rewriting the PR** — Don't suggest a complete rewrite. Point to the problem.
5. **Praising obvious things** — "Nice use of Result!" — skip it
6. **Reviewing what's not in the diff** — Stay focused on what changed

Pr Review

Works with

Security Analysis

Attribution

Comments (0)