review-pr
Scannednpx machina-cli add skill Roberdan/MyConvergio/review-pr --openclawPre-PR Review Skill
Focused pre-PR review targeting patterns that GitHub Copilot code review catches. NOT a generic code review — use
/code-reviewfor that.
Purpose
Catch the 9 recurring pattern categories (contract mismatch, null safety, error handling, scalability, security, logic errors, architecture drift, test quality, naming conflicts) BEFORE creating a PR, reducing review cycles.
When to Use
- Before
gh pr create— run/review-prfirst - After significant changes to a feature branch
- When touching API routes AND frontend consumers in same PR
Workflow
Step 1: Detect Scope
# Detect base branch
BASE=$(git merge-base --fork-point main HEAD 2>/dev/null || echo "main")
# Get changed files
CHANGED_FILES=$(git diff --name-only --diff-filter=ACMR "${BASE}...HEAD")
# Count and report scope
FILE_COUNT=$(echo "$CHANGED_FILES" | grep -c . || echo 0)
If no changed files detected, report "No changes found" and exit.
Step 2: Automated Pattern Check (Zero AI Cost)
Run the mechanical pattern checker first:
~/.claude/scripts/code-pattern-check.sh --diff-base "$BASE" --json
Parse the JSON output. Report P1/P2 counts immediately.
Step 3: Read Changed Files
Read the FULL content of each changed file (not just diff). Cross-file analysis requires full context.
Prioritize:
- API route files (
**/api/**/route.ts,**/api/**/route.py) - Service/hook files that consume APIs (
**/services/*.ts,**/hooks/use*.ts) - Type/interface definitions (
**/types/*.ts,**/interfaces/*.ts) - Test files (
**/*.test.ts,**/*.spec.ts,**/test_*.py)
Step 4: AI Cross-File Analysis
Analyze for patterns that code-pattern-check.sh CANNOT catch:
4a. Contract Mismatch (P1)
- Compare API route return types with frontend fetch/hook expectations
- Check Prisma schema field types vs API DTO fields
- Verify request body types match API parameter expectations
4b. Logic Errors (P1)
- Wrong field used in object mapping (e.g.,
user.namevsuser.displayName) - Regex patterns that match too broadly or too narrowly
- Off-by-one in array/string operations
- Transaction ordering (dependent ops outside
$transaction)
4c. Architecture Drift (P2)
- Read active ADRs:
ls docs/adr/*.mdand checkStatus: Accepted - Compare new code patterns against ADR decisions
- Flag contradictions (e.g., localStorage usage when ADR mandates Zustand)
4d. Test Quality (P2)
- Check test files cover edge cases, not just happy path
- Flag brittle assertions (
toHaveBeenCalledTimeson implementation details) - Verify error paths are tested
- Check for shared mutable state between tests
Step 5: Generate Report
Output a structured report with this format:
## Pre-PR Review: {branch_name}
**Scope**: {file_count} files changed | Base: {base_branch}
### Automated Findings (code-pattern-check.sh)
| Severity | Check | Count |
| -------- | ---------------------- | ----- |
| P1 | unguarded_json_parse | 2 |
| P2 | missing_error_boundary | 1 |
### AI Analysis Findings
| # | Severity | Category | File | Line | Issue |
| --- | -------- | ------------------ | ---------------------- | ---- | ------------------------------------------------- |
| 1 | P1 | contract_mismatch | src/api/users/route.ts | 45 | Returns `{name}` but hook expects `{displayName}` |
| 2 | P2 | architecture_drift | src/store/auth.ts | 12 | Uses localStorage, ADR-007 requires Zustand |
### Recommendations
1. **[P1]** Fix contract mismatch in user API...
2. **[P2]** Migrate auth storage to Zustand per ADR-007...
### Summary
- **P1 (must fix)**: {count}
- **P2 (should fix)**: {count}
- **P3 (consider)**: {count}
- **Verdict**: {READY | NOT READY} for PR
What This Skill Does NOT Do
- Generic code quality review (use
/code-review) - Style/formatting checks (use ESLint/Prettier)
- Performance profiling (use
/performance) - Security audit (use
/security-audit)
Reference
- Pattern knowledge base:
~/.claude/reference/copilot-patterns.md - Automated checker:
~/.claude/scripts/code-pattern-check.sh - Copilot feedback digest:
~/.claude/scripts/copilot-review-digest.sh - Full code review:
/code-reviewskill
Integration with Thor
Thor Gate 4b runs code-pattern-check.sh automatically during validation.
This skill adds AI analysis on top for pre-PR use.
Source
git clone https://github.com/Roberdan/MyConvergio/blob/master/.claude/skills/review-pr/SKILL.mdView on GitHub Overview
This skill performs a focused pre-PR review to catch GitHub Copilot patterns before you open a PR. It targets nine recurring categories—contract mismatch, null safety, error handling, security, scalability, logic errors, architecture drift, test quality, and naming conflicts—to reduce back-and-forth in reviews. By surfacing critical issues early, it helps teams ship safer, more reliable changes faster.
How This Skill Works
Technically, it first detects the scope of changes, then runs a zero-cost code-pattern checker, then reads full contents of changed files for cross-file analysis. It checks for patterns the checker cannot catch (contract mismatches, logic errors, architecture drift, test quality) and finally emits a structured report guiding reviewers before the PR is created.
When to Use It
- Before gh pr create — run /review-pr first
- After significant changes to a feature branch
- When touching API routes AND frontend consumers in the same PR
- During refactors that touch public API contracts
- Prior to merging hotfixes that affect APIs or data contracts
Quick Start
- Step 1: Detect scope and identify BASE and CHANGED_FILES
- Step 2: Run code-pattern-check.sh --diff-base BASE --json and parse results
- Step 3: Read full changed files, run AI cross-file analysis, and generate the Pre-PR report
Best Practices
- Run the mechanical code-pattern-checker first (P1/P2 counts surfaced immediately)
- Limit the PR scope to affected modules: API routes, services, types, tests
- Prioritize cross-file reading of API routes, services, types, and tests as defined
- Use the AI cross-file findings to fix contract mismatches and logic errors before PR
- Check ADRs and avoid architecture drift (e.g., avoid localStorage when ADR requires Zustand)
Example Use Cases
- PR editing src/api/users/route.ts and frontend fetch expectations
- PR updating Prisma DTOs without updating API DTOs leading to contract mismatch
- PR introducing a logic error in object mapping (user.name vs user.displayName)
- PR causing architecture drift by using localStorage vs Zustand per ADR
- PR with thin test coverage, missing edge-case tests flagged by Test Quality