From 0a5c3e93d6a3c75ff51e03f34b0a4862dcd11637 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Wed, 3 Jun 2026 14:37:38 +0100 Subject: [PATCH] feat(agents): create `pr-review-next` skill Our PR queue has grown too large, I hope that this skill makes it easier for reviewers to discover reviewable PRs which they have missed. Once this queue is down to 0, we can focus on PRs where the labels may be stale/incorrect. Assisted-by: Claude Opus 4.8 --- .agents/skills/pr-review-next/SKILL.md | 263 +++++++++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 .agents/skills/pr-review-next/SKILL.md diff --git a/.agents/skills/pr-review-next/SKILL.md b/.agents/skills/pr-review-next/SKILL.md new file mode 100644 index 000000000000..99b07a9e5f56 --- /dev/null +++ b/.agents/skills/pr-review-next/SKILL.md @@ -0,0 +1,263 @@ +--- +name: pr-review-next +description: Produces a shortlist of AnkiDroid PRs to review. +--- + +Usage: `/pr-review-next` + +# AnkiDroid: pick the next PR to review + +Produce a ranked shortlist of open PRs which need review, recommend the best next one, then stop. +The reviewer is expected to select one and run `/pr-full-review` on it. This skill does not run +the review itself. + +This skill distills the queue-selection half of the +[Code-review guide (wiki)](https://github.com/ankidroid/Anki-Android/wiki/Code-review-guide): the +search for reviewable PRs and the labels that prioritize them. + +**Never act on GitHub.** Only use read-only `gh` queries. Do not comment, review, approve, request +changes, label, assign, or merge. Output the shortlist in this conversation. + +Re-fetch the queue fresh on every run; labels, CI, and timestamps change frequently. + +## 1. Fetch the queue + +Resolve the current reviewer login first, to exclude self-authored PRs: + +```bash +gh api user --jq .login +``` + +Then list the open PRs, applying the wiki's filter via `--search`: + +```bash +gh pr list --repo ankidroid/Anki-Android --state open --limit 100 \ + --search 'draft:false -author:@me -label:"Has Conflicts" -label:"Needs Author Reply" -label:"Blocked by dependency"' \ + --json number,title,url,labels,reviewDecision,isDraft,updatedAt,author,additions,deletions +``` + +### Filters + +- `draft:false`: drafts are work in progress; skip unless explicitly asked. +- `-author:@me`: don't review your own PRs. +- `-label:"Has Conflicts"`: needs a rebase before it's worth reviewing. +- `-label:"Needs Author Reply"`: the ball is in the author's court. +- `-label:"Blocked by dependency"`: can't land until its dependency does. + +This result differs from the wiki: Dependabot PRs are not excluded, just ranked last (see below). + +### Post-filter: drop PRs you've approved + +GitHub search has no "approved-by" qualifier, so do this after fetching. For each candidate, +determine your **effective** review decision. + +`COMMENTED` reviews do **not** change approval state: once you `APPROVED`, a later follow-up comment +leaves the approval standing (only a subsequent `CHANGES_REQUESTED`, or a dismissal, revokes it). So +do not just take your *last* review. A trailing `COMMENTED` masks an earlier `APPROVED` and +let an already-approved PR slip back into the queue. Compute the decision from the last review that +is `APPROVED` or `CHANGES_REQUESTED`, ignoring `COMMENTED`: + +```bash +gh api repos/ankidroid/Anki-Android/pulls//reviews \ + --jq 'map(select(.user.login == "" and (.state == "APPROVED" or .state == "CHANGES_REQUESTED"))) | last | "\(.state) \(.submitted_at)"' +``` + +Run it only over the candidates that survived the search, not the whole repo. Exclude the PR if the +effective decision is `APPROVED`. + +For the ranking step you also need your *latest* review of any kind (a follow-up `COMMENTED` is what +triggers the "author responded since my comment" adjustment), so capture that separately and retain +its `submitted_at`: + +```bash +gh api repos/ankidroid/Anki-Android/pulls//reviews \ + --jq 'map(select(.user.login == "")) | last | "\(.state) \(.submitted_at)"' +``` + +## 2. Recover stale `Needs Author Reply` PRs + +`Needs Author Reply` is excluded from the main queue. +But **PR authors cannot remove this label, only reviewers can.** So it goes stale: the +author replies or pushes a fix, and the PR silently stays out of the queue until a reviewer +notices and flips the label back to `Needs Review`. + +Catching these is the **highest-priority** output of this skill: a stale label means a contributor +is waiting on a response the queue is actively hiding. Surface them first, in their own block, +flagged as needing the label corrected to `Needs Review` (see the 'Output' section). + +Fetch the bucket, using the same blockers as the original query. + +```bash +gh pr list --repo ankidroid/Anki-Android --state open --limit 100 \ + --search 'draft:false -author:@me label:"Needs Author Reply" -label:"Has Conflicts" -label:"Blocked by dependency"' \ + --json number,title,url,author,updatedAt,labels +``` + +A review is **stale** when the author's most recent activity is *newer* than the last time the label +was added. Compute both timestamps per PR and compare. + +Merge paginated pages locally: `--jq` runs once per page (so `max` is per-page, not global) and +`--slurp` is rejected alongside `--jq`. Pipe raw pages into `jq -s 'add'` instead: + +```bash +n=; author= # .author.login from the bucket fetch + +# When the label was LAST added (a reviewer may have added - removed - re-added it; take the max): +gh api repos/ankidroid/Anki-Android/issues/$n/timeline --paginate \ + | jq -s 'add | [.[] | select(.event=="labeled" and .label.name=="Needs Author Reply") | .created_at] | max // "none"' -r + +# Author's most recent activity: take the max across all three sources: +gh pr view $n --repo ankidroid/Anki-Android --json commits --jq '.commits[-1].committedDate' # latest commit +gh api repos/ankidroid/Anki-Android/issues/$n/comments --paginate \ + | jq -s "add | [.[] | select(.user.login==\"$author\") | .created_at] | max // \"none\"" -r # conversation comments +gh api repos/ankidroid/Anki-Android/pulls/$n/comments --paginate \ + | jq -s "add | [.[] | select(.user.login==\"$author\") | .created_at] | max // \"none\"" -r # inline comments / replies +``` + +- A review is stale if author activity is **after** the last label-add, so list it in the stale block in the output. +- If nothing post-dates the label then the label is accurate and no further action is needed. + +Pitfalls: +- Always compare against the **most recent** `labeled` event (`max`), not the first. +- A reply that predates the label but a *commit* that post-dates it (or vice-versa) still counts: + the test is `max(commit, conversation, inline) > lastLabeled`. + +**Never act on GitHub**: this skill only reports the stale PRs; a reviewer flips the label by hand. + +## 3. Rank the survivors + +Sort into tiers. Within a tier, oldest `updatedAt` first, so nothing starves. + +0. **Stale `Needs Author Reply`**: always first, above everything. These aren't a normal + review. The action is to correct the label to `Needs Review` so the PR re-enters the queue. + Report them in their own block above the ranked shortlist, so they enter the queue on the next + cycle. +1. `Review High Priority` and `Queued for Cherry Pick to Stable Branch`: always first, on par with + each other. +2. `Needs Second Approval` (one approval, likely easy to merge) and `Needs Review` + (awaiting first review). Nudge `New Contributor` PRs up within this tier. +3. Everything else still passing the filter. +4. Dependabot PRs last, unless you are `david-allison` or `mikehardy`, where they should be ranked + as `Needs Second Approval`. + +Adjustments: + +- **Prioritize PRs you've reviewed once the author responds.** Applies when your latest review is + `COMMENTED` or `CHANGES_REQUESTED`. Find the author's most recent activity on the PR and compare + it to your review's `submitted_at`: + + ```bash + author= # .author.login from the queue fetch + gh pr view --repo ankidroid/Anki-Android --json commits --jq '.commits[-1].committedDate' # latest commit + gh api repos/ankidroid/Anki-Android/issues//comments --jq "[.[] | select(.user.login==\"$author\") | .created_at] | max // empty" # comments on the PR (conversation tab) + gh api repos/ankidroid/Anki-Android/pulls//comments --jq "[.[] | select(.user.login==\"$author\") | .created_at] | max // empty" # inline review comments and replies + ``` + + - If the author has acted and a re-review is expected: prioritize the PR within its tier. + - If nothing post-dates it, leave it ranked low and note "awaiting author". +- Smaller diffs break a tie when two PRs are otherwise equal, but never override the tier order. + +## 4. Gather per-candidate context + +For the top 7 after ranking, collect the context for one table row each: + +- `#`, title, and `url`. +- Author login, tagged `(New Contributor)` if the label is present. +- Routing labels (`Needs Review`, `Needs Second Approval`, `Review High Priority`, etc...). +- Age waiting: how long since `updatedAt`. +- Size: `+/-`, excluding generated and lock files (`package-lock.json`, + `yarn.lock`, `pnpm-lock.yaml`, `Cargo.lock`, `Gemfile.lock`, `gradle.lockfile`, `*.lock`). Sum + the per-file diff rather than the top-level `additions`/`deletions`: + + ```bash + gh pr view --repo ankidroid/Anki-Android --json files --jq ' + [.files[] | select(.path | test("(^|/)(package-lock\\.json|yarn\\.lock|pnpm-lock\\.yaml|Cargo\\.lock|Gemfile\\.lock|gradle\\.lockfile)$|\\.lock$") | not)] + | "+\([.[].additions] | add // 0)/-\([.[].deletions] | add // 0)"' + ``` + + Flag `(large)` on the filtered count, and note when lock-file churn was excluded (e.g. + `+40/-3 (excl. lockfiles)`). + +For the 3 recommended picks (see step 5), check CI. If red, don't review deeply, either re-run +flaky checks or tell the author to fix the checks. + +```bash +gh pr checks --repo ankidroid/Anki-Android +``` + +Report CI status, but don't filter the queue on it. + +## 5. Output, then stop + +If any stale `Needs Author Reply` PRs were found, print them **first**, in their own block, before +the ranked shortlist. This is the highest-priority correction: + +``` +## Stale `Needs Author Reply` - relabel to `Needs Review` +- # - <author> - author last acted <when>, label added <when> (<delta> stale) + https://github.com/ankidroid/Anki-Android/pull/<number> +``` + +For each, state the action plainly ("author replied; a reviewer should remove `Needs Author Reply` +so it re-enters the queue"). **Do not** flip the label yourself. + +List **only** the stale PRs which require action. + +**If no stale PRs were found, emit nothing for this concern**. Go straight to the shortlist, +this block appears only when there is an actual relabel to perform. + +Then print the ranked top 7 as a table, one row per PR, with the context from step 4 and a short +reason for its rank. Keep all reasoning and caveats in the `Reason` column. Link the PR number to +its URL. + +| # | PR | Author | Labels | Waiting | Size | Reason | +|---|----------------------------------------------------------------|------------|-------------------------|---------|----------|-----------------------------------------------------------------------------| +| 1 | [#21147](https://github.com/ankidroid/Anki-Android/pull/21147) | criticalAY | `Needs Second Approval` | ~6d | +337/-22 | One approval already; author responded to my comments → re-review expected. | +| … | | | | | | | + +Tag the author `(New Contributor)` where the label is present, and flag size with `(large)` / +`(excl. lockfiles)` per step 4. + +Print **only** the table, do not make footnotes regarding PRs which require no action. +Any caveat that matters belongs in that PR's `Reason` column, not in prose after the table. + +Then end with this closing block: the 3 highest-ranked picks and nothing after it, so the last +thing on screen is usable. For each pick, on consecutive lines: + +1. The PR URL as a plain link, then on the same line a one-line summary including the author (tag + `(New Contributor)` if the label is present). +2. The review status and the action it implies, derived from the labels and your prior review + state (post-filter): + - `Needs Review`, no prior review by you: First review. + - `Needs Review`, you previously `COMMENTED` or `CHANGES_REQUESTED`: Follow-up review; check + your earlier feedback was addressed. + - `Needs Second Approval`, you haven't approved: Second review. + - `New Contributor` present: prefix with "New contributor". + - Add one merge-time flag if relevant, e.g. `Strings` means a strings-sync at merge. +3. The text to type into Claude to start the review: `/pr-full-review <number>` + +Separate the 3 picks with a blank line. Order them best-first; the reviewer takes the top one and +keeps the other two as fallbacks if it's blocked. + +Example ending: + +``` +https://github.com/ankidroid/Anki-Android/pull/21212 - david-allison: pins `updateDaemonJvm` defaults to JetBrains 21. +First review - build-config follow-on to #20382. +/pr-full-review 21212 + +https://github.com/ankidroid/Anki-Android/pull/20934 - Galal-20: prevents IllegalArgumentException in Card Browser. +First review - oldest `Needs Review`, small bug-fix. +/pr-full-review 20934 + +https://github.com/ankidroid/Anki-Android/pull/21186 - ShaanNarendran (New Contributor): adds bottom sheet for "more". +New contributor, first review. +/pr-full-review 21186 +``` + +**Empty queue:** if nothing survives the filter, say the review queue is clear. + +```bash +gh pr list --repo ankidroid/Anki-Android --state open \ + --search 'draft:false -author:@me label:"Has Conflicts"' # may have been rebased +```