Skip to content

Assign external-PR reviewers deterministically, not randomly - #372

Open
noahho wants to merge 1 commit into
mainfrom
noah/deterministic-pr-reviewer
Open

Assign external-PR reviewers deterministically, not randomly#372
noahho wants to merge 1 commit into
mainfrom
noah/deterministic-pr-reviewer

Conversation

@noahho

@noahho noahho commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Replaces the random reviewer pick with a deterministic one keyed on the PR number. The file stays as the source of reviewers.

-const reviewer = reviewers[Math.floor(Math.random() * reviewers.length)];
+const reviewer =
+  reviewers[context.payload.pull_request.number % reviewers.length];

Why

Dependabot opens its PRs as a run of consecutive numbers, and Math.random() happily gives the same person several in a row - one reviewer drew both #368 and #369 in tabpfn-extensions. Modulo makes consecutive numbers structurally distinct.

Measured against the real eligible-PR history (fork + Dependabot PRs, 8 reviewers):

TabPFN (59 PRs) tabpfn-extensions (101 PRs)
Back-to-back repeats, modulo 3 of 58 3 of 100
Back-to-back repeats, random 8 13
Consecutive-number pairs landing on the same reviewer 0 of 27 0 of 40

What this is not: a round robin. Long-run counts are about as uneven as random (modulo spread 4-10 across 8 buckets on TabPFN, 8-16 on tabpfn-extensions; random baseline 4-11 and 8-18), because the PR numbers that reach this workflow are sparse - it fires only for forks and Dependabot, and PR numbers share a counter with issues. Equal counts per reviewer would need stored state or GitHub's own team-based review assignment. This fixes the clustering, not the fairness, and the comment in the workflow says so.

Side benefit: it is idempotent. A reopen or ready-for-review re-request now names the same person instead of possibly adding a second reviewer.

Same change is going into the other copy of this workflow (TabPFN and tabpfn-extensions are the only two repos that have it).

🤖 Generated with Claude Code

Math.random() gave one reviewer several consecutive Dependabot PRs (#368/#369 in
tabpfn-extensions). Keying on the PR number makes consecutive numbers land on
different reviewers, and makes a rerun idempotent. It is a spread rather than a
true round robin - long-run counts stay as uneven as random - which the comment
in the workflow now states.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 06:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the external/dependabot PR reviewer assignment workflow to choose reviewers deterministically (based on PR number) instead of randomly, reducing clustering for consecutive Dependabot PRs while keeping the reviewer list file as the single source of truth.

Changes:

  • Replace Math.random()-based reviewer selection with PR_number % reviewers.length.
  • Update workflow step naming and inline rationale/comments to reflect deterministic selection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +63 to +67
// Deterministic rather than random, keyed on the PR number. Dependabot
// opens its PRs in a run of consecutive numbers, so consecutive numbers
// must not collide: n % len guarantees that, which random did not (one
// reviewer drew two of #368/#369 in tabpfn-extensions). It is also
// idempotent, so a reopen re-requests the same person rather than adding
runs-on: ubuntu-slim
steps:
- name: Request review from a random reviewer
- name: Request review from the rotation
@noahho noahho added the no changelog needed PR has no user-facing impact; skip the changelog gate label Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog needed PR has no user-facing impact; skip the changelog gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants