Skip to content
Open
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
59 changes: 59 additions & 0 deletions .claude/skills/review-prs/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
name: review-prs
tools: Read, Grep, Glob, Bash
---

Review all open PRs on pyinfra-dev/pyinfra and maintain review files in `.prs/`:

## Steps

1. **Run sync script**: Execute `bash .claude/skills/review-prs/sync.sh` to clean up stale reviews and get the list of PRs to update/create.

2. **For each PR** in the sync output (Update + New only, NOT Unchanged), launch agents in parallel (batches of 3-5) to review:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
2. **For each PR** in the sync output (Update + New only, NOT Unchanged), launch agents in parallel (batches of 3-5) to review:
2. **For each PR** in the sync output (Update + New only, NOT Unchanged), launch agents using rolling concurrency (see Parallelism below) to review:

- Each agent gets: repo name, PR number, whether it's an update or new review.
- Agent fetches the PR diff via `gh pr diff --repo pyinfra-dev/pyinfra {number}`.
- Agent also fetches `updatedAt` via `gh pr view --repo pyinfra-dev/pyinfra {number} --json updatedAt --jq .updatedAt` and includes it in the review header as `**PR Updated:** {timestamp}`.
- Agent reads relevant source code files being modified (use Read/Grep, never assume).
- Agent writes the review file to `.prs/{number}.md`.

3. **Review format** for each PR file (`.prs/{number}.md`):

```markdown
# PR #{number} - {title}

**Author:** {author}
**URL:** https://github.com/pyinfra-dev/pyinfra/pull/{number}
**Review Date:** {YYYY-MM-DD}
**PR Updated:** {updatedAt ISO timestamp from GH API}

## Verdict: {GO | NO-GO | NEEDS-DISCUSSION}

{1-2 sentence summary of what the PR does}

## Issues

{Only if there are problems. Each issue should be a concise bullet with a code snippet if relevant. Skip this section entirely if no issues.}

## Notes

{Optional minor observations, nits, or suggestions. Keep brief.}
```

## Review guidelines

- **Be minimal**: Only highlight actual problems or risks. Don't pad reviews with praise or restatements.
- **Code snippets**: When referencing problematic code, include the relevant snippet (keep short).
- **Deep inspection**: Always read the actual source code being modified. Never assume behavior - trace code paths, check callers, verify types. Use Grep/Read extensively.
- **Verdict options**:
* **GO** = safe to merge as-is or with trivial nits only
* **NO-GO** = has bugs, security issues, breaking changes, or significant design problems
* **NEEDS-DISCUSSION** = requires maintainer input on approach/design

## Parallelism

- Process multiple PRs in parallel using Agent tool where possible (batch into groups of 3-5).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Process multiple PRs in parallel using Agent tool where possible (batch into groups of 3-5).
Use **rolling concurrency**, not fixed batches. The goal is to keep ~5 PR reviews in flight at all times so a slow review never blocks a fast one.
- Spawn each PR-review agent with `run_in_background: true` so completions arrive as individual notifications instead of blocking on a whole batch.
- **Initial fill**: in a single message, launch up to 5 background agents (one per PR) to saturate the in-flight pool.
- **On each completion notification**: if any PRs remain in the queue, immediately launch one new background agent to replace the finished one — keep the in-flight count at 5 until the queue is empty.
- Track three sets in your head (or via TaskCreate if it helps): `queued` (not yet started), `in_flight` (background agents running), `done` (review file written). Move PRs between sets as agents complete.
- Do not wait for the full pool to drain before launching replacements — the whole point is to avoid that.

- Each agent should be given the full context: repo name, PR number, review format, and instruction to deeply inspect code.

## Final Summary

Once all review updates are complete, output a summary table of PRs grouped by verdict. Re-read the PR files to collect updated verdicts for each.
88 changes: 88 additions & 0 deletions .claude/skills/review-prs/sync.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#!/usr/bin/env bash
# Syncs .prs/ review files against open GitHub PRs.
# Deletes reviews for merged/closed PRs, outputs a plan for what to update/create.
# Skips re-review when PR hasn't been updated since the last review.
set -euo pipefail

REPO="pyinfra-dev/pyinfra"
PRS_DIR="$(git rev-parse --show-toplevel)/.prs"

mkdir -p "$PRS_DIR"

# Fetch all open PR numbers (updated in last year)
SINCE=$(date -v-1y +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || date -d '1 year ago' +%Y-%m-%dT%H:%M:%SZ)
OPEN_PRS=$(gh pr list --repo "$REPO" --state open --limit 200 --json number,title,author,updatedAt \
--jq "[.[] | select(.updatedAt >= \"$SINCE\")] | sort_by(.number)")

OPEN_NUMBERS=$(echo "$OPEN_PRS" | jq -r '.[].number')

# Find existing review files
EXISTING=()
for f in "$PRS_DIR"/*.md; do
[ -f "$f" ] || continue
num=$(basename "$f" .md)
[[ "$num" =~ ^[0-9]+$ ]] && EXISTING+=("$num")
done

# Delete stale reviews (merged/closed)
DELETED=()
for num in "${EXISTING[@]}"; do
if ! echo "$OPEN_NUMBERS" | grep -qx "$num"; then
rm "$PRS_DIR/$num.md"
DELETED+=("$num")
fi
done

# Categorize: update (changed), unchanged (skip), or new
UPDATE=()
UNCHANGED=()
NEW=()
while IFS= read -r num; do
if [ -f "$PRS_DIR/$num.md" ]; then
# Compare stored updatedAt with current PR updatedAt
stored_ts=$(sed -n 's/^\*\*PR Updated:\*\* //p' "$PRS_DIR/$num.md" 2>/dev/null || echo "")
current_ts=$(echo "$OPEN_PRS" | jq -r ".[] | select(.number == $num) | .updatedAt")
if [ -n "$stored_ts" ] && [ "$stored_ts" = "$current_ts" ]; then
UNCHANGED+=("$num")
else
UPDATE+=("$num")
fi
else
NEW+=("$num")
fi
done <<< "$OPEN_NUMBERS"

# Output markdown summary
echo "# PR Review Sync"
echo ""
echo "**Repo:** $REPO"
echo "**Date:** $(date +%Y-%m-%d)"
echo ""

if [ ${#DELETED[@]} -gt 0 ]; then
echo "## Deleted (merged/closed)"
for num in "${DELETED[@]}"; do
echo "- #$num"
done
echo ""
fi

if [ ${#UNCHANGED[@]} -gt 0 ]; then
echo "## Unchanged since last review (${#UNCHANGED[@]})"
echo "$OPEN_PRS" | jq -r --argjson nums "$(printf '%s\n' "${UNCHANGED[@]}" | jq -R 'tonumber' | jq -s '.')" \
'.[] | select(.number as $n | $nums | index($n)) | "- #\(.number) \(.title) (@\(.author.login))"'
echo ""
fi

echo "## Update existing reviews (${#UPDATE[@]})"
if [ ${#UPDATE[@]} -gt 0 ]; then
echo "$OPEN_PRS" | jq -r --argjson nums "$(printf '%s\n' "${UPDATE[@]}" | jq -R 'tonumber' | jq -s '.')" \
'.[] | select(.number as $n | $nums | index($n)) | "- #\(.number) \(.title) (@\(.author.login))"'
fi
echo ""

echo "## New reviews needed (${#NEW[@]})"
if [ ${#NEW[@]} -gt 0 ]; then
echo "$OPEN_PRS" | jq -r --argjson nums "$(printf '%s\n' "${NEW[@]}" | jq -R 'tonumber' | jq -s '.')" \
'.[] | select(.number as $n | $nums | index($n)) | "- #\(.number) \(.title) (@\(.author.login))"'
fi
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,5 @@ docs/_deploy_globals.rst

pyinfra-debug.log
Makefile

.prs/
Loading