feat(refresh-reviewers): recompute the reviewer expertise map from git history and open a drift PR (BE-4116)#63
Conversation
…xpertise map from git history, open a drift PR (BE-4116)
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 10 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 5 |
| 🟢 Low | 5 |
Panel: 8/8 reviewers contributed findings.
…I-failure no-op, branch guards, stale-PR reconcile, md escaping (BE-4116) Addresses the cursor-review panel findings on #63: - validate HALF_LIFE_DAYS strictly positive (zero divided by zero, negative inverted the decay) — falls back to the default with a warning - distinguish API failure from "no linked account" via an API_ERROR sentinel; any failed email->login lookup downgrades the run to the documented clean no-op instead of silently dropping contributors - workflow: refuse pr_branch == default branch, add a per-repo concurrency group, and never force-reset an open drift PR whose tip a human authored - workflow: a genuine no-drift run (report present) closes a stale still-bot-authored drift PR so an obsolete proposal can't linger mergeable - workflows_ref is now required (defaulting to mutable main defeated the pin-by-SHA rule); no consumers exist yet so nothing breaks - gap report: accumulate once per commit per key (parity with rule scores) and key by top-two-level DIRECTORY, never the filename - decode git's C-style path quoting properly (tabs, quotes, octal) - escape | and backticks in repo-derived text in the PR-body tables - document the accepted trust boundary (forgeable author metadata, bounded by human review) and the caller-trusted EXTRA_EXCLUDE_PATHS regexes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ELI-5
When someone opens a PR,
assign-reviewers.ymlpicks owners from a hand-written map (.github/reviewers.yml) of "who knows which part of the code." That map goes stale as people join, leave, or change focus. This PR adds a sibling reusable workflow,refresh-reviewers.yml, that a repo can run on a schedule: it re-reads the repo's git history, re-scores who actually works where (recent commits count more; bots, lockfiles, and generated code don't count), and — only when the map has drifted — opens ONE small PR proposing the updated lists, with the scores shown so a human can sanity-check and merge. It never assigns anyone and never merges anything itself.What's here
.github/workflows/refresh-reviewers.yml— the reusableworkflow_callengine, followingassign-reviewers.yml's conventions: app-token auth (vars.APP_ID+CLOUD_CODE_BOT_PRIVATE_KEY), caller-side config, never-fail defensive handling. The workflow's ownpermissions:staycontents: read; the branch push + PR open go through the app token. Generator script is loaded from a pinnedworkflows_refof this repo (tamper-proofing convention), never the caller's checkout..github/refresh-reviewers/generate.py— the engine, ported from the validated BE-4114 prototype (not redesigned): decayed commit-touch scoring (0.5^(age/half_life), line counts intentionally unused), bot-author + generated/churn-path + rename-syntax filtering, noreply-email decoding (incl.digits+login@) with per-run-cached commits-API fallback, collaborator (not org-member) eligibility, per-rule threshold/floor/backfill selection with the under-floor leave-unchanged cold-start fallback,default_pool= top-5 overall minus ≥2-rule anchors minusmap_exclude, a surgical rewrite that touches only thereviewers:/default_pool:sequences (flow AND block) and preserves every comment byte-for-byte, plus the report-only taxonomy-gap section.bot/refresh-reviewersfrom the default branch, commit the single file, force-push, and create the PR or edit the existing open one's body — duplicates never stack..github/refresh-reviewers/tests/test_generate.py— 38 pure-python tests (repo.github/<tool>/tests/pattern): glob parity withglobToRegExp(**/inference/**vsservices/ingest/**cases), decay math, threshold/floor/backfill/under-floor selection, bot/path/rename filtering, noreply decoding, byte-preserving rewrite (flow + block + scalar), PR-body content. Plustest-refresh-reviewers.yml(path-filtered CI, mirrorstest-cursor-review-scripts.yml)..github/refresh-reviewers/README.md(contract, knob defaults + BE-4114 rationale,map_excludeguidance), root README table row, AGENTS.md entries (137/200 lines, integrity checker passes).Verification
python3 -m unittest discover -s .github/refresh-reviewers/tests— 38/38 pass; all other suites (cursor-review, agents-md-integrity, groom, bump-callers) still green;actionlintclean on both new workflows.Judgment calls
workflows_refinput added (not in the ticket contract): required by this repo's "scripts are the single source of truth, loaded from a pinned ref" convention — same shape asagents-md-integrity.yml/pr-size.yml.::warning::; genuine bugs still raise so they're visible. Collaborator-API-down aborts quietly rather than emitting a map with unvalidated eligibility.DrJKL-style) would be silently dropped. Regression-tested.old => newwithout braces) is normalized too, not just the ticket's brace form — git emits both.bot/refresh-reviewersis per the ticket's reset-branch idempotency spec; the branch is bot-owned by contract. Noconcurrency:group (siblings don't set one; weekly-schedule overlap is negligible).