Skip to content

ci: add risk-based PR review routing and auto-approval #2108

Description

@hemildesai

Use cases, pain points, and background

NeMo Gym already runs automated Claude review on eligible internal PRs (#1622, #1878), but every PR follows the same path and the verdict is not a machine-readable approval policy.

Amplitude's risk-based review system and PyTorch's AI coding playbook suggest the same goal: remove friction from changes already easy to review without lowering the merge standard.

Description

Add a versioned PR risk check with three outcomes:

  • Low: eligible for automatic approval after all safety gates pass.
  • Medium/high: automated linting plus required human/owner review.
  • Unknown/error: fail closed to human review.

Risk signals should include author trust and ownership, diff size and scope, touched components, tests, public API/schema/security impact, dependencies/workflows, mixed cosmetic and semantic changes, and NeMo-critical areas such as verifier/reward logic, rollout collection, and async/Ray behavior.

Deterministic rules set the minimum tier. Subsystem-specific Claude lint checks may raise risk or block approval, but never lower it or replace human accountability.

A low-risk PR may be auto-approved only when its author is a trusted maintainer who already has approval rights for every affected area; required CI is green; Claude reports SHIP with no BLOCKER/RISK findings; no review threads, change requests, or owner reviews are outstanding; and every signal is bound to the current head SHA.

Medium/high-risk changes should route to the relevant owner. Give owners a review window, then allow normal human review after the existing SLA so an unavailable owner cannot block indefinitely.

Design

  • Store thresholds, protected paths, ownership rules, and overrides in a reviewed policy file.
  • Reuse the repository's existing docs/server/core CI classification.
  • Define AI lint criteria per subsystem for ownership, tests, public APIs, security, and global invariants.
  • Reassess on every head change and publish the tier, reasons, policy version, and SHA.
  • Run the privileged controller from trusted base-branch code; never execute PR code with write credentials or secrets.
  • Provide human escalation, an auditable override, an owner-review window, and a repository kill switch.
  • Start in shadow mode and retain misclassifications as regression fixtures.
  • Keep auto-merge disabled initially; enable it separately only after auto-approval is proven safe.

Out of scope

  • Replacing human ownership with AI review.
  • Auto-approving external/fork PRs or changes outside the author's ownership.
  • Bypassing CI, branch protection, owners, or unresolved reviews.
  • Auto-merging medium/high/unknown-risk changes.

Acceptance Criteria

  • A tested, versioned policy produces a risk tier and reasons for every PR SHA.
  • Auto-approval requires author trust and approval rights for every affected area.
  • Protected, broad, mixed-purpose, public-API, and classifier-error cases require human review.
  • Subsystem-specific AI lint checks can only maintain or raise the deterministic risk tier.
  • New commits invalidate stale assessments; all gates are rechecked immediately before approval.
  • Relevant owners receive a review window with a documented SLA fallback.
  • The privileged workflow uses trusted code and least privilege; decisions and overrides are auditable.
  • Shadow-mode results are reviewed before enforcement, with unsafe cases retained as regression tests.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-designGoal is clear, but needs input on technical designrepo-toolingCI/CD, build, tooling improvements

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions