refresh-reviewers

refresh-reviewers — recompute the reviewer expertise map from git history

The engine behind refresh-reviewers.yml: a scheduled drift-detector that recomputes the caller repo's .github/reviewers.yml (the config assign-reviewers.yml consumes at PR time) from git history and opens one reviewable single-file PR when the committed map has fallen behind reality. It never assigns anyone and never merges anything — the drift PR is the whole deliverable, and a human accepts or edits it.

How it scores

Per commit on the default branch within the window, per rule bucket touched by ≥1 surviving changed file:

score[rule][login]   += 0.5 ** (age_days / half_life_days)
touches[rule][login] += 1

• Buckets are the committed rules. Each rules: entry's path globs are the bucket definition, matched with the same glob semantics as assign-reviewers.yml's globToRegExp (* within a segment, ** across, ? one non-slash code POINT) — the map is only correct if it is scored with the matcher the runtime assigns with.
• Line counts are intentionally unused. Recency-decayed commit touches are the signal; numstat is read only for the changed-file list.
• Excluded: bot authors ([bot]@ emails, noreply@argoproj.io), generated/churn paths (ent codegen outside ent/schema/, *.gen.go, *.pb.go, vendor/, Go sum files, JS/generic lockfiles, dynamicconfig version bumps, frontend-version.json, plus caller extra_exclude_paths regexes), and rename noise (numstat's {old => new} resolves to the new path).
• Emails → logins: login@users.noreply.github.com (and the digits+login@ form) decode directly; every other distinct email costs one GET /repos/{owner}/{repo}/commits/{sha} to read .author.login. Commits whose email has no linked account are dropped and the count is reported in the PR body — but an API failure (rate limit, 5xx, timeout) is never scored as "no account": any failed lookup downgrades the whole run to the clean no-op, so a transient outage can't emit a biased map.
• Eligibility = repo collaborators (paginated GET .../collaborators) — not org members, because addAssignees silently drops non-collaborators, so the collaborator set is the exact test the runtime applies.

Selection

Per rule: contributors with touches >= min_touches and score >= min_score, ranked by score, capped at top_k. Below floor qualifiers, the ranked remainder backfills (needing only touches >= floor_min_touches); a rule still under the floor is left unchanged and noted in the PR body — the runtime already falls back to default_pool when a rule can't match, so a cold bucket keeps its hand-set owners. default_pool becomes the top-5 whole-repo scorers minus anyone already anchoring ≥2 rules (the anti-pile-on rationale from ComfyUI_frontend#5448) minus map_exclude.

The rewrite is surgical: only the reviewers: [...] / default_pool: [...] sequences (flow or block) are replaced; every comment and all other bytes — the document's own line endings included — are preserved. The config's comments are its documentation — a YAML-dump rewrite would be a regression. The committed blob is read as bytes and decoded explicitly (never with text=True, whose universal-newline translation would silently rewrite a CRLF config to LF), inserted block items copy — per position — the line ending of the item they displace, and the proposal is written with newline="" so the runner OS cannot translate anything on the way out. A config that is not valid UTF-8, or one written with CR-only line breaks (which the byte-faithful reader does not split on, so it would arrive as a single line), takes the documented clean no-op rather than a lossy or half-read rewrite.

Two shapes the rewrite deliberately cannot manage, both left exactly as committed rather than edited:

• an inline trailing comment on a block-form list item (- alice # why) is not carried over when that list's membership changes — keep per-reviewer rationales on their own comment lines;
• a multi-line flow sequence (reviewers: [alice, closing on a later line). Every edit the rewrite makes is confined to the one line a location names, so rewriting the opening line would leave the continuation lines behind as orphaned YAML. Those lists are parsed but never rewritten; each one emits a ::warning:: naming its file and line and is listed in the drift PR's body. Put the list on one line to let the refresher manage it. A torn paths: holds back its rule's reviewers: for a different reason: that list is editable, but the rule's globs were truncated at the line break, so anything selected from them would be scored against the wrong bucket — a wrong write rather than a skipped one.

A list the rewrite skips is reported as skipped everywhere, not just in the bytes: report.json marks it skipped and never changed, its before/after row shows the committed list (read across the continuation lines, so the before column is the real one and not the parser's truncated view), and it still counts toward the default pool's anti-pile-on tally.

The PR body carries the per-rule before/after table with scores/touches, the unresolved-email count, the knob values, and a report-only taxonomy gap section: the hottest top-two-level directories matched by no rule glob, with their top contributors — candidates for new rules, never auto-added.

Knob defaults (and why)

knob	default	rationale
window_months	12	Long enough to cover slow-moving subsystems; validated in BE-4114 against a caller repo's history.
half_life_days	90	The decay is what correctly aged out stale expertise (e.g. inference contributors who had moved on); 90d matched observed team reality where flat counts did not.
top_k / floor	4 / 2	Enough experts to load-balance across without piling every rule onto the same two people.
min_touches / min_score	5 / 1.5	Filters drive-by contributors: one huge recent commit is not sustained expertise.
floor_min_touches	2	Relaxed bar used only to reach the floor.
pr_branch	bot/refresh-reviewers	Stable branch, force-reset each run — re-runs update the one open drift PR instead of stacking duplicates. Guarded: it may never equal the default branch, a tip a human has pushed to is never force-reset ("edit freely" means it), and a no-drift run closes a stale still-bot-authored PR.

map_exclude guidance

map_exclude controls who may appear in the committed map; it is distinct from the runtime vars.REVIEWER_EXCLUDE (who assign-reviewers skips at PR time). Seed it with operator logins whose commit volume is largely agent-authored — their history signal is machine throughput, not personal review expertise, and without the exclusion they would anchor every bucket.

Files

• generate.py — the whole engine (stdlib-only; parsing, scoring, selection, surgical rewrite, report/PR-body emission). Loaded at run time from a pinned ref of this repo, never from the caller's checkout.
• tests/test_generate.py — pure-python tests: glob parity, decay math, threshold/floor/backfill selection, bot/path/rename filtering, noreply decoding, byte-preserving rewrite. Its TestSharedParserCorpus drives ../assign-reviewers/parser-corpus.json — the same fixture ../assign-reviewers/tests/assignment.test.cjs runs through the JS originals — so parser/glob parity is executable rather than a comment. Only the config half of parse_reviewer_config's (config, locations) return is compared there; locations is Python-only and stays covered by the rewrite tests. Add parser cases to the corpus, never as an inline literal. Run:

python3 -m unittest discover -s .github/refresh-reviewers/tests -p 'test_*.py' -v