Skip to content

Add shadow-mode PR risk classifier - #25

Merged
tnsardesai merged 7 commits into
mainfrom
hypeship/pr-risk-classifier
Oct 27, 2026
Merged

tnsardesai merged 7 commits into
mainfrom
hypeship/pr-risk-classifier

Conversation

@tnsardesai

@tnsardesai tnsardesai commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds pr-risk.yml, a reusable workflow that comments on small PRs to say whether they would have been auto-approved and why. It never approves. This is a two-week shadow run, so we can measure how many PRs would qualify, whether they were really low risk, and how much review wait they would have saved, before deciding on real auto-approval.

Flow (on opened, ready_for_review and synchronize; drafts are skipped):

  1. Gather (scripts/pr_risk.py gather, caller's read-only token). The PR is skipped with no comment if it is a draft, comes from a fork, changes more than 50 lines, touches a deny-listed path (defaults in the script plus the caller's deny-paths), or has a file with no text diff. Otherwise it builds the model input: title and description, the diff, ±30 line-numbered lines of head content around each hunk, and AGENTS.md read from the base branch so the PR cannot rewrite it.
  2. Classify. claude -p --tools "" runs in an empty directory with --json-schema. The prompt treats all PR content as untrusted. It defines zero/low/medium/high tiers and a deny list the model applies on its own judgment (auth, secrets, security controls, billing, compliance groups like BAA/ZDR, schema and data deletion, CI/CD, capacity and routing, removed safety checks, anything it can't understand). The model step only sees ANTHROPIC_API_KEY.
  3. Comment (scripts/pr_risk.py comment). The kernel-internal token is minted after the model step, scoped to the current repo with pull-requests: write only. It writes one sticky comment under 100 words. "Would auto-approve" means not denied, tier zero or low, and confidence not low. The comment ends with a hidden pr-risk:v1 JSON marker holding the verdict and per-commit history. A later push that makes the PR ineligible rewrites an existing comment to "no", so a stale "yes" never stays up. A run whose head commit has moved does not post.

Secrets are named rather than inherited, matching breakglass-merge.yml.

Testing

  • scripts/test_pr_risk.py, 14 tests, run by the new pr-risk-tests.yml. Covers the deny-list globs, every reason a PR is ineligible, silent skips, creating vs. updating the comment, rewriting a stale comment, ignoring marker comments from people, classifier failures, a head that moved mid-run, the <100-word limit, how the model input is built (base-branch guidance, tag scrubbing, context ranges), the gh api calls the client makes (via a stub gh), and the workflow's credential layout. All existing tests still pass. actionlint is clean.
  • Dry run against 18 real merged PRs in infra, kernel, kernel-images-private and hypeman, using the real gather and model call but printing the comment instead of posting. 6 would auto-approve: flag allowlist adds, a narrow error-mapping fix, and Stainless SHA syncs. 11 would not: compliance-group and credential-feature flag changes, regions-to-all, site-isolation flag removals, a CORS change on a vault endpoint, cert and Chromium bumps. 1 was skipped as a draft. Comments were 53–70 words, at about $0.22–0.35 per run on opus.
  • Not tested end to end: the app-token and posting path in Actions. It is covered by the unit tests' fake GitHub, and will be exercised by the first caller PR.

Note

Medium Risk
Adds CI/CD and PR-comment automation with GitHub App write access and external script fetch; shadow-only today, but it shapes future auto-approval policy and handles sensitive PR metadata via the model API.

Overview
Introduces a shadow-mode PR risk classifier as a reusable pr-risk.yml workflow plus scripts/pr_risk.py, so consumer repos can see whether small PRs would have been auto-approved—without ever approving or merging.

Eligibility is enforced before any model call: skip silently for drafts, forks, non–org-member authors, >50 changed lines, deny-listed paths (lockfiles, .github/**, secrets patterns, etc., with optional deny-paths), or files without a text diff. Eligible PRs get a gather → classify → comment pipeline: build untrusted-tagged model input (diff, base-branch AGENTS.md, surrounding line context), run Claude Code with no tools and only ANTHROPIC_API_KEY, then post or update one bot comment via a repo-scoped GitHub App token minted only after classification (pull-requests: write).

Comments state Would auto-approve: yes/no from tier (zero/low + not deny + confidence not low), embed hidden pr-risk:v1 JSON for analytics, rewrite stale “yes” when a PR later becomes ineligible, and avoid races when HEAD moves mid-run.

Also adds classify-prompt.md, pr-risk-tests.yml (unittest + PyYAML), README consumer wiring, and 14 unit tests covering globs, ineligibility, comment lifecycle, prompt injection scrubbing, and workflow credential isolation.

Reviewed by Cursor Bugbot for commit cbd706d. Bugbot is set up for automated code reviews on this repo. Configure here.

@tnsardesai
tnsardesai marked this pull request as ready for review October 27, 2026 04:19
@tnsardesai

Copy link
Copy Markdown
Contributor Author

Picked sjmiller609 to review these related PRs (#25, kernel/infra#1825, kernel/kernel#5021, kernel/kernel-images-private#517, kernel/hypeman#531): recent commits in workflows; 2 pending

Comment thread README.md
Comment on lines +70 to +77
Shadow-mode risk check for small PRs. When a PR is opened or marked ready, and on every push,
it leaves one comment saying whether the PR would have been auto-approved and why. It never
approves.

These PRs are skipped with no comment: drafts, fork PRs, PRs over 50 changed lines, and PRs
that touch a deny-listed path. The defaults live in `scripts/pr_risk.py`, and callers can add
more through `deny-paths`. Claude classifies everything else and can deny a PR on its own
judgment. The comment stores its verdict in a hidden `pr-risk:v1` JSON marker for later analysis.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It doesn't seem like it would be too expensive because we skip anything over 50 lines.

I'm wondering if this use case seems right for Jev kind of model, it would just be a lot faster maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call on cost, it should stay cheap since everything over 50 lines is skipped before the model runs.

I'll test Jev before we turn on auto-approve after the two-week shadow run. By then we'll have a labeled set (each verdict plus 👍/👎 reactions and what reviewers actually did), so I can run Jev and Claude on the same PRs and compare them directly.

For now I think Claude is the better fit:

  • The hard part is the deny check, not the yes/no. Getting it right means reading the diff and the code around it to work out what a change actually does. For example, a one-line flag edit that adds an org to the BAA group, or a dropped site-per-process entry that turns out to be a sandboxing control. Claude caught both in the dry runs, and that kind of reasoning seems like a stretch for a model that returns probabilities.
  • Speed doesn't matter much here. The comment is posted in the background and nobody waits on it.
  • The reason is the useful output during the shadow run. Reviewers judge a verdict by the sentence explaining it. That's what the 👍/👎 reactions measure, and it's what we'll go back to in two weeks.

Where Jev could be a good fit is phase 2. Its probabilities would give us a tunable approval threshold, possibly with Claude still writing the reason or handling the uncertain cases.

@tnsardesai
tnsardesai merged commit 7865174 into main Oct 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants