From c0f3db1d0005d98cec26133bc1b1f3e0a6d58377 Mon Sep 17 00:00:00 2001 From: Iuri Oksuzian Date: Sat, 8 Aug 2026 11:09:38 -0500 Subject: [PATCH] =?UTF-8?q?skills:=20add=20pr-digest=20=E2=80=94=20read-on?= =?UTF-8?q?ly=20PR=20landscape=20across=20the=20nine=20offline=20repos?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reviewing-pull-requests answers "is this one PR any good?". Nothing answered "what is the state of the queue?" — which PRs are unreviewed, which have red CI, and which ones I reviewed before the author pushed again. pr-digest is strictly read-only: every call is a GET. It never posts, comments, triggers a build, edits or merges. That is what makes it cheap to trust; anything needing action gets handed to reviewing-pull-requests. Reports open non-draft PRs with author, age, CI state at the *current* head, overall review decision, and whether your own review covers that head. Flags five conditions: your review is stale (head moved after you reviewed), CI red at head, no CI at head, never reviewed by anyone, and merge conflict. "No CI at head" is raised only for Offline and Production. FNALbuild does not watch the other seven repos, so a blank CI column there is normal rather than a finding. Failure handling is loud: an unknown repo is a hard error, and a repo that fails to report is named in an INCOMPLETE section with the totals labelled a lower bound -- never silently rendered as zero PRs. Two gh quirks are documented in the skill so they are not "optimized" back: gh pr list --json latestReviews returns commit.oid as an empty string (hence one reviews call per PR to get commit_id, which the whole stale-review check depends on), and latestReviews also carries the full review body, ~15 KB per PR of prose the digest never displays. Co-Authored-By: Claude Opus 5 (1M context) --- skills/pr-digest/SKILL.md | 112 ++++++++++++++ skills/pr-digest/scripts/pr_digest.py | 201 ++++++++++++++++++++++++++ 2 files changed, 313 insertions(+) create mode 100644 skills/pr-digest/SKILL.md create mode 100755 skills/pr-digest/scripts/pr_digest.py diff --git a/skills/pr-digest/SKILL.md b/skills/pr-digest/SKILL.md new file mode 100644 index 0000000..4555a5f --- /dev/null +++ b/skills/pr-digest/SKILL.md @@ -0,0 +1,112 @@ +--- +name: pr-digest +description: Landscape view of open pull requests across the Mu2e offline repos — who authored what, review state, CI state at the current head, and which PRs need your attention. Use when asked for a PR summary, PR status, "what's open on Offline", or what needs reviewing. Read-only. +compatibility: Requires an authenticated `gh` CLI and network access to github.com. Makes no writes. +metadata: + version: "1.0.0" + last-updated: "2026-08-08" +--- + +# PR Digest + +## Purpose + +Answer "what is the state of the PR queue?" — as opposed to +`reviewing-pull-requests`, which answers "is this one PR any good?". + +Use it for: a standup-style summary, deciding what to review next, +finding PRs whose head moved after you reviewed them, and spotting CI +that is red or was never run at the current head. + +## Read-only + +Every call this skill makes is a GET. It never posts a review, never +comments, never triggers a build, never edits or merges anything. That +is the point: a digest is cheap to trust precisely because it cannot +change anything. If a PR in the digest needs action, hand it to +`reviewing-pull-requests` — do not act from here. + +## Scope + +The nine Mu2e offline repos: `Offline`, `Production`, `EventNtuple`, +`EventDisplay`, `DQM`, `Tutorial`, `PassN`, `RefAna`, `ArtAnalysis`. +Passing any other repo is a hard error, not a silent skip. + +Open, non-draft PRs only. Drafts are excluded — they are not asking for +review yet. + +## Usage + +``` +python3 scripts/pr_digest.py # all nine repos +python3 scripts/pr_digest.py Offline # one repo +python3 scripts/pr_digest.py Offline Production +python3 scripts/pr_digest.py --json # machine-readable +``` + +Roughly 15 s for all nine repos (~20 API calls for ~12 open PRs). +Exit status is 1 if any repo failed to report, 0 on a clean run — so a +caller can distinguish a partial picture from a complete one. + +## What it reports + +**NEEDS ATTENTION** — only PRs with at least one flag, most-flagged +first. Nothing here means nothing is flagged, which is stated explicitly +rather than left as an empty section. + +| flag | meaning | why it matters | +|---|---|---| +| `your review is STALE` | you reviewed at sha X, head is now Y | your findings describe code that no longer exists; the author pushed after you | +| `CI RED at head` | a check reports FAILURE at the current head | names the failing contexts, so you can tell a real break from an infra blip | +| `no CI at head` | Offline/Production PR with no check at this head | the green you see may belong to an older commit — see `reviewing-pull-requests` §Triggering a CI Build | +| `never reviewed by anyone` | no `reviewDecision` and no review by you | the queue's actual backlog | +| `merge CONFLICT` | `mergeable == CONFLICTING` | blocked regardless of review state | + +**ALL OPEN** — one row per PR: repo#number, author, age in days, CI +mark, overall review decision, and your own review state. + +The `you` column is the one to read: `@head` means your review covers +the current head, a sha8 means your latest review is at that older +commit, `-` means you have not reviewed it. + +CI marks: `ok` green, `FAIL` red, `run` pending, `--` no checks. + +**INCOMPLETE** — any repo that did not report cleanly is listed by name +with its error, and the totals are labelled a lower bound. A repo that +errors is never silently rendered as zero PRs. + +## Interpreting `--` in the CI column + +`--` is normal and not a finding on `EventNtuple`, `EventDisplay`, +`DQM`, `Tutorial`, `PassN`, `RefAna` and `ArtAnalysis` — FNALbuild does +not watch those repos, so there is no CI to be missing. The digest only +raises "no CI at head" for `Offline` and `Production`, where the absence +is genuinely informative. + +## Implementation notes + +Two `gh` quirks drove the call structure. Do not "optimize" them away: + +- **`gh pr list --json latestReviews` returns `commit.oid` as an empty + string.** The whole point of the `you` column is comparing your + review's commit against the live head, so the digest makes one + `gh api .../pulls//reviews` call per PR to get `commit_id`. There is + no way to get it from the list endpoint. +- **`latestReviews` also carries the full review `body`.** On a PR with + a long review that is ~15 KB of prose per PR, for a field the digest + never displays. `latestReviews` is deliberately absent from the + requested field list. + +A PR is treated as covered if *any* of your reviews sits at the current +head, not merely the most recent one — posting a 🟡 comment after an +earlier 🔴 at the same sha should not read as stale. + +## What it deliberately does not do + +- **No quality judgement.** It reports that CI is red, not why. It + reports that a PR is unreviewed, not whether it is any good. +- **No unanswered-question detection.** Deciding whether a reviewer's + inline question was actually answered needs to read the thread and + judge it — that is review work, and it belongs in + `reviewing-pull-requests`, not in a digest that must stay cheap. +- **No writes.** See above. diff --git a/skills/pr-digest/scripts/pr_digest.py b/skills/pr-digest/scripts/pr_digest.py new file mode 100755 index 0000000..64b9cfb --- /dev/null +++ b/skills/pr-digest/scripts/pr_digest.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +"""Read-only landscape view of open PRs across Mu2e offline repos. + +Makes no writes of any kind. Every gh call is a GET. + +Usage: + pr_digest.py [repo ...] # default: all nine skill-scope repos + pr_digest.py --json # machine-readable, no table + pr_digest.py Offline # one repo +""" + +import json +import subprocess +import sys +from datetime import datetime, timezone + +REPOS = ["Offline", "Production", "EventNtuple", "EventDisplay", "DQM", + "Tutorial", "PassN", "RefAna", "ArtAnalysis"] + +# FNALbuild only watches these two; elsewhere "no CI" is normal, not a finding. +FNALBUILD_REPOS = {"Offline", "Production"} + +PR_LIMIT = 100 # if a repo ever exceeds this we say so rather than truncate silently + +LIST_FIELDS = ("number,title,author,createdAt,updatedAt,headRefOid," + "reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,url,isDraft") + + +def gh(args): + """Run a gh command, returning (stdout, error_or_None). Never raises.""" + try: + p = subprocess.run(["gh"] + args, capture_output=True, text=True, timeout=120) + except (OSError, subprocess.TimeoutExpired) as e: + return "", f"{type(e).__name__}: {e}" + if p.returncode != 0: + return "", (p.stderr or "").strip().splitlines()[-1] if p.stderr else f"exit {p.returncode}" + return p.stdout, None + + +def me(): + out, err = gh(["api", "user", "--jq", ".login"]) + if err: + sys.exit(f"FATAL: cannot determine gh user ({err}). Is gh authenticated?") + return out.strip() + + +def ci_summary(rollup, repo): + """Collapse statusCheckRollup into (state, detail). State in + green/red/pending/none.""" + if not rollup: + return ("none", "no checks at head" if repo in FNALBUILD_REPOS else "n/a") + fail, pend, ok = [], [], 0 + for c in rollup: + # StatusContext uses state; CheckRun uses conclusion/status. + st = (c.get("state") or c.get("conclusion") or c.get("status") or "").upper() + name = c.get("context") or c.get("name") or "?" + if st in ("FAILURE", "ERROR", "TIMED_OUT", "CANCELLED"): + fail.append(name) + elif st in ("PENDING", "IN_PROGRESS", "QUEUED", "EXPECTED", ""): + pend.append(name) + else: + ok += 1 + if fail: + return ("red", ", ".join(sorted(fail)[:3]) + ("…" if len(fail) > 3 else "")) + if pend: + return ("pending", ", ".join(sorted(pend)[:3]) + ("…" if len(pend) > 3 else "")) + return ("green", f"{ok} checks") + + +def my_review(repo, num, user, head8): + """Latest review by `user`: (state, sha8, at_head) or None. One GET.""" + out, err = gh(["api", f"repos/Mu2e/{repo}/pulls/{num}/reviews", "--paginate", + "--jq", f'.[] | select(.user.login=="{user}") ' + '| "\\(.state)\\t\\(.commit_id[0:8])\\t\\(.submitted_at)"']) + if err: + return ("ERROR", err[:40], False) + rows = [r.split("\t") for r in out.strip().splitlines() if r.strip()] + if not rows: + return None + rows.sort(key=lambda r: r[2]) # chronological; last is most recent + state, sha, _ = rows[-1] + # Any review at the live head counts as covered, even if an older one was later. + at_head = any(r[1] == head8 for r in rows) + return (state, sha, at_head) + + +def collect(repos, user): + prs, errors = [], [] + for repo in repos: + out, err = gh(["pr", "list", "--repo", f"Mu2e/{repo}", "--state", "open", + "--limit", str(PR_LIMIT), "--json", LIST_FIELDS]) + if err: + errors.append(f"{repo}: {err}") + continue + try: + items = json.loads(out or "[]") + except json.JSONDecodeError as e: + errors.append(f"{repo}: unparseable gh output ({e})") + continue + if len(items) >= PR_LIMIT: + errors.append(f"{repo}: hit the {PR_LIMIT}-PR cap; list may be truncated") + for it in items: + if it.get("isDraft"): + continue + head8 = (it.get("headRefOid") or "")[:8] + ci, ci_detail = ci_summary(it.get("statusCheckRollup"), repo) + mine = my_review(repo, it["number"], user, head8) + created = datetime.fromisoformat(it["createdAt"].replace("Z", "+00:00")) + prs.append({ + "repo": repo, "number": it["number"], "title": it["title"], + "author": (it.get("author") or {}).get("login", "?"), + "age_days": (datetime.now(timezone.utc) - created).days, + "head": head8, "url": it["url"], + "review_decision": it.get("reviewDecision") or "", + "mergeable": it.get("mergeable") or "", + "merge_state": it.get("mergeStateStatus") or "", + "ci": ci, "ci_detail": ci_detail, + "my_review_state": mine[0] if mine else "", + "my_review_sha": mine[1] if mine else "", + "my_review_at_head": bool(mine and mine[2]), + }) + return prs, errors + + +def attention(pr, user): + """Reasons this PR wants a human. Order matters: most actionable first.""" + out = [] + if pr["my_review_state"] and not pr["my_review_at_head"]: + out.append(f"your review is STALE (reviewed {pr['my_review_sha']}, head {pr['head']})") + if pr["ci"] == "red": + out.append(f"CI RED at head — {pr['ci_detail']}") + if pr["ci"] == "none" and pr["repo"] in FNALBUILD_REPOS: + out.append(f"no CI at head {pr['head']}") + if not pr["review_decision"] and not pr["my_review_state"]: + out.append("never reviewed by anyone") + if pr["mergeable"] == "CONFLICTING": + out.append("merge CONFLICT") + return out + + +CI_MARK = {"green": "ok", "red": "FAIL", "pending": "run", "none": "--"} + + +def render(prs, errors, user, repos): + W = 78 + print(f"Mu2e PR digest — {datetime.now().strftime('%Y-%m-%d %H:%M')} — as {user}") + print(f"{len(repos)} repo(s), {len(prs)} open non-draft PR(s)") + print("=" * W) + + flagged = [(p, attention(p, user)) for p in prs] + flagged = [(p, r) for p, r in flagged if r] + if flagged: + print("\nNEEDS ATTENTION") + for p, reasons in sorted(flagged, key=lambda x: (-len(x[1]), x[0]["repo"])): + print(f" {p['repo']}#{p['number']} {p['title'][:52]}") + for r in reasons: + print(f" - {r}") + print(f" {p['url']}") + else: + print("\nNEEDS ATTENTION: nothing flagged.") + + print("\nALL OPEN") + hdr = f" {'PR':<20} {'author':<16} {'age':>4} {'CI':<5} {'review':<16} {'you':<10} title" + print(hdr) + print(" " + "-" * (len(hdr) - 2)) + for p in sorted(prs, key=lambda x: (x["repo"], -x["number"])): + you = ("@head" if p["my_review_at_head"] + else (p["my_review_sha"] if p["my_review_state"] else "-")) + dec = (p["review_decision"] or "-").replace("CHANGES_REQUESTED", "CHANGES_REQ") + print(f" {p['repo']+'#'+str(p['number']):<20} {p['author'][:16]:<16} " + f"{str(p['age_days'])+'d':>4} {CI_MARK[p['ci']]:<5} {dec[:16]:<16} " + f"{you:<10} {p['title'][:28]}") + + if errors: + print("\nINCOMPLETE — these repos did not report cleanly:") + for e in errors: + print(f" ! {e}") + print(" Treat counts above as a lower bound, not a complete picture.") + + +def main(): + argv = [a for a in sys.argv[1:] if a != "--json"] + as_json = "--json" in sys.argv[1:] + repos = argv or REPOS + bad = [r for r in repos if r not in REPOS] + if bad: + sys.exit(f"FATAL: not a skill-scope repo: {', '.join(bad)}\n" + f"Known: {', '.join(REPOS)}") + user = me() + prs, errors = collect(repos, user) + if as_json: + print(json.dumps({"user": user, "repos": repos, "prs": prs, + "errors": errors}, indent=2)) + else: + render(prs, errors, user, repos) + # Exit 1 if any repo failed, so a caller can tell a partial run from a clean one. + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main())