[PR #78] [MERGED] ci(assign-reviewers): self-enroll this repo + seed the expertise map from approval history #90

New issue

Closed

opened 2026-10-26 22:36:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:36:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/78
Author: @mattmillerai
Created: 7/26/2026
Status: ✅ Merged
Merged: 7/26/2026
Merged by: @mattmillerai

Base: main ← Head: matt/self-enroll-assign-reviewers

📝 Commits (2)

5dd39e7 ci(assign-reviewers): self-enroll this repo + seed the expertise map from approval history
7f6259e ci(assign-reviewers): skip fork PRs cleanly and close two expertise-map glob gaps

📊 Changes

3 files changed (+210 additions, -3 deletions)

View changed files

➕ .github/reviewers.yml (+131 -0)
➕ .github/workflows/ci-assign-reviewers.yml (+76 -0)
📝 AGENTS.md (+3 -3)

📄 Description

17 open PRs in this repo had no assignee, so nothing routed them to anyone who could approve them. This repo has shipped an expertise-aware, load-balanced router since BE-1943 and never enrolled itself in it.

Worth stating up front, because the name misleads: assign-reviewers.yml writes the ASSIGNEE field, not reviewer requests. From its own header — "this org routes/alerts people via the ASSIGNEE field". That is precisely the routing that was missing here.

Why the map is seeded from approvals, not git history

Nearly every commit on main here has a single author, so authorship carries no routing signal at all. Scoring it would put one name in every bucket. The buckets are instead populated from who has actually approved merged PRs in the matching area:

approver	merged-PR approvals	area
wei-hai	9	bump-callers, CI plumbing, `assign-*`
huntcsg	3	cursor-review internals
deepme987	3	bump-callers wire-bot, assign-reviewers
sundar-svg	2	cursor-review `runs_on` / self-hosted
synap5e	1	groom (builder PR body, BE-4346)
luke-mino-altherr	1	groom (direct-CLI agent phases, BE-4214)
bigcat88	1	groom (caller permissions)

This is a prerequisite for #63, not a duplicate of it

refresh-reviewers (#63, BE-4116) treats the committed rules: globs as its bucket definitions and only re-scores membership — it cannot bootstrap a map from nothing. Two warnings are recorded in the file itself:

It scores recency-decayed commit authorship, so on this repo its first drift PR will want to collapse buckets toward the single dominant committer. That is a history artifact, not a routing improvement — read that PR, don't rubber-stamp it.
The globs are therefore the durable part of the file. They outlive the names.

`default_pool` is load-bearing

Selection drops the PR author, so a bucket whose only member is the author empties. default_pool is three proven approvers and deliberately excludes the person who opens most PRs here.

Verification

The reusable parses reviewers.yml with a hand-rolled minimal YAML parser (js-yaml isn't available inside github-script), so standard YAML validity is not sufficient. I extracted that parser and its globToRegExp / matchesAny verbatim from assign-reviewers.yml and drove the committed map through them:

7 rules, every one with non-empty paths + reviewers; default_pool parsed correctly.
17 representative repo paths all route to the intended bucket; an unmatched path correctly falls through to default_pool.
Retro-validated against the 17 live assignments: 15/17 of the assignees fall inside what this map would pick, and 0 fell through to default_pool.
actionlint clean; all 8 referenced logins confirmed repo collaborators (GitHub silently drops an unassignable login, so this is worth checking rather than assuming).

The two deviations are deliberate human overrides, not map errors:

PR	assigned	map would pick	why
#71	guill	groom bucket	routed as a privilege-boundary review, not a groom-domain one
#61	sundar-svg	guill, wei-hai	pure load-spreading on a one-line binary deletion

That retro pass is also what caught a real gap: ci-assign-reviewers.yml does not match assign-*.yml, so this repo's own routing caller would have fallen through to default_pool. It now names itself.

num_reviewers: 1 matches how PRs here are actually tracked — one named owner, not a pair. Easy to raise if single-owner PRs start stalling.

ELI5

Pull requests here were piling up with nobody's name on them, so nobody knew they were meant to look. The repo already had a robot that puts the right person's name on a PR based on which files it touches — it had just never been switched on for itself.

This switches it on and gives it the phone book. The phone book is built from who has actually approved this kind of change before, not from who wrote the code, because one person wrote nearly all the code here and that tells you nothing about who should check it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/78 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/26/2026 **Status:** ✅ Merged **Merged:** 7/26/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/self-enroll-assign-reviewers` --- ### 📝 Commits (2) - [`5dd39e7`](https://github.com/Comfy-Org/github-workflows/commit/5dd39e7e19c53010a80b9bc641743840f067c262) ci(assign-reviewers): self-enroll this repo + seed the expertise map from approval history - [`7f6259e`](https://github.com/Comfy-Org/github-workflows/commit/7f6259ec0a97fd04d0ad1c6b88a589c0fa00a08b) ci(assign-reviewers): skip fork PRs cleanly and close two expertise-map glob gaps ### 📊 Changes **3 files changed** (+210 additions, -3 deletions) <details> <summary>View changed files</summary> ➕ `.github/reviewers.yml` (+131 -0) ➕ `.github/workflows/ci-assign-reviewers.yml` (+76 -0) 📝 `AGENTS.md` (+3 -3) </details> ### 📄 Description 17 open PRs in this repo had no assignee, so nothing routed them to anyone who could approve them. This repo has shipped an expertise-aware, load-balanced router since BE-1943 and never enrolled itself in it. Worth stating up front, because the name misleads: **`assign-reviewers.yml` writes the ASSIGNEE field, not reviewer requests.** From its own header — *"this org routes/alerts people via the ASSIGNEE field"*. That is precisely the routing that was missing here. ## Why the map is seeded from approvals, not git history Nearly every commit on `main` here has a single author, so **authorship carries no routing signal at all**. Scoring it would put one name in every bucket. The buckets are instead populated from who has actually **approved merged PRs** in the matching area: | approver | merged-PR approvals | area | |---|---|---| | wei-hai | 9 | bump-callers, CI plumbing, `assign-*` | | huntcsg | 3 | cursor-review internals | | deepme987 | 3 | bump-callers wire-bot, assign-reviewers | | sundar-svg | 2 | cursor-review `runs_on` / self-hosted | | synap5e | 1 | groom (builder PR body, BE-4346) | | luke-mino-altherr | 1 | groom (direct-CLI agent phases, BE-4214) | | bigcat88 | 1 | groom (caller permissions) | ## This is a prerequisite for #63, not a duplicate of it `refresh-reviewers` (#63, BE-4116) treats the **committed `rules:` globs as its bucket definitions** and only re-scores membership — it cannot bootstrap a map from nothing. Two warnings are recorded in the file itself: 1. It scores recency-decayed **commit authorship**, so on this repo its first drift PR will want to collapse buckets toward the single dominant committer. That is a history artifact, not a routing improvement — read that PR, don't rubber-stamp it. 2. The globs are therefore the durable part of the file. They outlive the names. ## `default_pool` is load-bearing Selection drops the PR author, so **a bucket whose only member is the author empties**. `default_pool` is three proven approvers and deliberately excludes the person who opens most PRs here. ## Verification The reusable parses `reviewers.yml` with a **hand-rolled minimal YAML parser** (js-yaml isn't available inside `github-script`), so standard YAML validity is not sufficient. I extracted that parser *and* its `globToRegExp` / `matchesAny` verbatim from `assign-reviewers.yml` and drove the committed map through them: - **7 rules**, every one with non-empty `paths` + `reviewers`; `default_pool` parsed correctly. - **17 representative repo paths** all route to the intended bucket; an unmatched path correctly falls through to `default_pool`. - **Retro-validated against the 17 live assignments: 15/17** of the assignees fall inside what this map would pick, and **0** fell through to `default_pool`. - `actionlint` clean; all 8 referenced logins confirmed repo collaborators (GitHub silently drops an unassignable login, so this is worth checking rather than assuming). The two deviations are deliberate human overrides, not map errors: | PR | assigned | map would pick | why | |---|---|---|---| | #71 | guill | groom bucket | routed as a privilege-boundary review, not a groom-domain one | | #61 | sundar-svg | guill, wei-hai | pure load-spreading on a one-line binary deletion | That retro pass is also what caught a real gap: **`ci-assign-reviewers.yml` does not match `assign-*.yml`**, so this repo's own routing caller would have fallen through to `default_pool`. It now names itself. `num_reviewers: 1` matches how PRs here are actually tracked — one named owner, not a pair. Easy to raise if single-owner PRs start stalling. ## ELI5 Pull requests here were piling up with nobody's name on them, so nobody knew they were meant to look. The repo already had a robot that puts the right person's name on a PR based on which files it touches — it had just never been switched on for itself. This switches it on and gives it the phone book. The phone book is built from who has actually approved this kind of change before, not from who wrote the code, because one person wrote nearly all the code here and that tells you nothing about who should check it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:36:06 +00:00