Skip to content

Commit 50a4098

Browse files
jithunnair-amdCopilotcursoragent
authored
Autolabel bot: optional draft field for path-based labeler.yml rules (#8020)
## Summary Extends repo `labeler.yml` so each label rule can opt out of (or limit to) draft pull requests. Path-based labels can stay on the legacy list-of-globs form; new behavior uses a small object with `globs` and optional `draft`. When a PR's draft state changes, the bot also **removes** draft-gated labels that no longer apply (e.g. `draft: false` labels when converted to draft, `draft: true` labels when marked ready for review). Removal is limited to labels the bot previously applied, so manually added labels are left alone. Logic lives in `labelerConfigUtils.ts`; `autoLabelBot.ts` imports it and re-exports `getLabelsFromLabelerConfig` so existing imports keep working. ## Motivation torchtitan CI uses [`labeler.yml`](https://github.com/pytorch/torchtitan/blob/main/.github/labeler.yml) to apply labels that trigger CI on PRs. We want to avoid adding those labels automatically on draft PRs to limit CI workloads, and remove them if a PR is converted back to draft. --- ## `labeler.yml` schema **Legacy (unchanged):** ```yaml "module: dynamo": - torch/_dynamo/** ``` **Extended:** ```yaml "ciflow/inductor": globs: - torch/_inductor/** draft: false ``` | `draft` | Behavior | | -------- | -------- | | (omitted) | Same as today: apply when paths match (draft or not). | | `false` | Apply only when the PR is not a draft. | | `true` | Apply only when the PR is a draft. | ## Code changes | Area | Change | | -------- | -------- | | `torchci/lib/bot/labelerConfigUtils.ts` | New module: parsing, `getLabelsFromLabelerConfig`, and `getDraftGatedLabelsToRemove`. | | `torchci/lib/bot/autoLabelBot.ts` | Pass `pull_request.draft`; handle `ready_for_review` and `converted_to_draft`; remove bot-applied draft-gated labels on draft transitions; skip required-label comments on `ready_for_review`. | | `torchci/lib/bot/utils.ts` | No changes on this branch. | ## Tests added / updated - `torchci/test/labelerConfigUtils.test.ts` — unit tests for parsing, draft gating, and label removal helpers. - `torchci/test/autoLabelBot.test.ts` — integration tests for adding labels on draft vs non-draft PRs, removing labels on `converted_to_draft` / `ready_for_review`, and no duplicate required-label comments on `ready_for_review`. ## Test plan - [x] From `torchci`, run: `jest test/labelerConfigUtils.test.ts` - [x] Run path-based labeler integration tests: `jest test/autoLabelBot.test.ts -t "labeler.yml config"` - [x] (Recommended) full auto-label suite: `jest test/autoLabelBot.test.ts` Authored with assistance from Cursor --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a6fa78f commit 50a4098

5 files changed

Lines changed: 682 additions & 30 deletions

File tree

torchci/lib/bot/autoLabelBot.ts

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
import { minimatch } from "minimatch";
21
import { Context, Probot } from "probot";
3-
import { addLabelErrComment, hasRequiredLabels } from "./checkLabelsUtils";
2+
import {
3+
addLabelErrComment,
4+
hasRequiredLabels,
5+
isBotAuthor,
6+
} from "./checkLabelsUtils";
7+
import {
8+
getDraftGatedLabelsToRemove,
9+
getLabelsFromLabelerConfig,
10+
} from "./labelerConfigUtils";
411
import {
512
addLabels,
613
CachedIssueTracker,
@@ -13,6 +20,8 @@ import {
1320
LabelToLabelConfigTracker,
1421
} from "./utils";
1522

23+
export { getLabelsFromLabelerConfig };
24+
1625
// List of regex patterns for assigning labels to both Pull Requests and Issues
1726
const IssueAndPRRegexToLabel: [RegExp, string][] = [
1827
[/rocm/gi, "module: rocm"],
@@ -140,27 +149,6 @@ const notUserFacingPatterns: RegExp[] = [
140149

141150
const notUserFacingPatternExceptions: RegExp[] = [/tools\/autograd/g];
142151

143-
export async function getLabelsFromLabelerConfig(
144-
context: Context,
145-
labelerConfigTracker: CachedLabelerConfigTracker,
146-
changed_files: string[]
147-
): Promise<string[]> {
148-
const config = await labelerConfigTracker.loadLabelsConfig(context);
149-
150-
const labels = [];
151-
152-
for (const [label, globs] of Object.entries(config)) {
153-
if (
154-
globs.some((glob: string) =>
155-
changed_files.some((file: string) => minimatch(file, glob))
156-
)
157-
) {
158-
labels.push(label);
159-
}
160-
}
161-
return labels;
162-
}
163-
164152
export async function getLabelsFromLabelToLabelConfig(
165153
context: Context,
166154
labelToLabelConfigTracker: LabelToLabelConfigTracker,
@@ -357,6 +345,39 @@ function getReleaseNotesCategoryAndTopic(
357345
return ["uncategorized", topic];
358346
}
359347

348+
async function getBotAppliedLabels(
349+
context: Context,
350+
owner: string,
351+
repo: string,
352+
issueNumber: number,
353+
labelNames: string[]
354+
): Promise<Set<string>> {
355+
if (labelNames.length === 0) {
356+
return new Set();
357+
}
358+
const lastLabeledBy = new Map<string, string>();
359+
const events = await context.octokit.paginate(
360+
context.octokit.issues.listEventsForTimeline,
361+
context.repo({
362+
issue_number: issueNumber,
363+
per_page: 100,
364+
})
365+
);
366+
for (const event of events) {
367+
if (event.event === "labeled" && event.label?.name) {
368+
lastLabeledBy.set(event.label.name, event.actor?.login ?? "");
369+
}
370+
}
371+
const botApplied = new Set<string>();
372+
for (const label of labelNames) {
373+
const actor = lastLabeledBy.get(label) ?? "";
374+
if (isBotAuthor(actor)) {
375+
botApplied.add(label);
376+
}
377+
}
378+
return botApplied;
379+
}
380+
360381
export async function wasLabelRecentlyRemoved(
361382
context: Context,
362383
issueNumber: number,
@@ -504,7 +525,13 @@ function myBot(app: Probot): void {
504525
});
505526

506527
app.on(
507-
["pull_request.opened", "pull_request.edited", "pull_request.synchronize"],
528+
[
529+
"pull_request.opened",
530+
"pull_request.edited",
531+
"pull_request.synchronize",
532+
"pull_request.ready_for_review",
533+
"pull_request.converted_to_draft",
534+
],
508535
async (context) => {
509536
const owner = context.payload.repository.owner.login;
510537
if (!isPyTorchbotSupportedOrg(owner)) {
@@ -547,10 +574,13 @@ function myBot(app: Probot): void {
547574
}
548575
}
549576

577+
const isDraft = context.payload.pull_request.draft;
578+
550579
var labelsFromLabelerConfig = await getLabelsFromLabelerConfig(
551580
context,
552581
labelerConfigTracker,
553-
filesChanged
582+
filesChanged,
583+
isDraft
554584
);
555585
labelsToAdd.push(...labelsFromLabelerConfig);
556586

@@ -579,10 +609,38 @@ function myBot(app: Probot): void {
579609

580610
await addNewLabels(labels, labelsToAdd, context);
581611

612+
const labelerConfig = await labelerConfigTracker.loadLabelsConfig(
613+
context
614+
);
615+
const draftGatedToRemove = getDraftGatedLabelsToRemove(
616+
labelerConfig as Record<string, unknown>,
617+
filesChanged,
618+
isDraft
619+
).filter((label) => labels.includes(label));
620+
const botAppliedLabels = await getBotAppliedLabels(
621+
context,
622+
owner,
623+
repo,
624+
context.payload.pull_request.number,
625+
draftGatedToRemove
626+
);
627+
for (const label of draftGatedToRemove) {
628+
if (!botAppliedLabels.has(label)) {
629+
continue;
630+
}
631+
await context.octokit.issues.removeLabel(
632+
context.repo({
633+
issue_number: context.payload.pull_request.number,
634+
name: label,
635+
})
636+
);
637+
}
638+
582639
// After auto-labeling is complete, check if the PR still needs required labels.
583640
// We do this here instead of in checkLabelsBot to avoid a race condition where
584641
// checkLabelsBot posts an error comment before auto-labeling has a chance to
585-
// add the required labels.
642+
// add the required labels. Only run on opened (not ready_for_review) so draft
643+
// transitions do not post duplicate required-label comments.
586644
if (
587645
isPyTorchPyTorch(owner, repo) &&
588646
context.payload.action === "opened"

torchci/lib/bot/checkLabelsUtils.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ export const LABEL_COMMENT_END = "\n<!-- check-labels-comment-end -->";
77
// Bot authors that can create the label error comment
88
export const BOT_AUTHORS = ["github-actions", "pytorchmergebot", "pytorch-bot"];
99

10+
export function isBotAuthor(author: string): boolean {
11+
return BOT_AUTHORS.includes(author.toLowerCase().replace("[bot]", ""));
12+
}
13+
1014
// Error message title
1115
export const LABEL_ERR_MSG_TITLE = "This PR needs a `release notes:` label";
1216

@@ -50,10 +54,7 @@ export function formLabelErrComment(): string {
5054
* Check if a comment is the label error comment.
5155
*/
5256
export function isLabelErrComment(body: string, author: string): boolean {
53-
return (
54-
body.includes(LABEL_COMMENT_START) &&
55-
BOT_AUTHORS.includes(author.toLowerCase().replace("[bot]", ""))
56-
);
57+
return body.includes(LABEL_COMMENT_START) && isBotAuthor(author);
5758
}
5859

5960
/**
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { minimatch } from "minimatch";
2+
import { Context } from "probot";
3+
import { CachedLabelerConfigTracker } from "./utils";
4+
5+
/** Legacy rules are a list of path globs; extended rules add optional draft behavior. */
6+
export type LabelerRule =
7+
| string[]
8+
| {
9+
globs: string[];
10+
/** false = skip label while PR is draft; true = only when draft; omit = always apply when globs match */
11+
draft?: boolean;
12+
};
13+
14+
export function labelerRuleSkipReason(
15+
rawRule: unknown
16+
): "invalid_draft" | "invalid_shape" | null {
17+
if (
18+
rawRule !== null &&
19+
typeof rawRule === "object" &&
20+
"globs" in rawRule &&
21+
Array.isArray((rawRule as { globs: unknown }).globs) &&
22+
(rawRule as { globs: unknown[] }).globs.every((x) => typeof x === "string")
23+
) {
24+
const r = rawRule as { globs: string[]; draft?: unknown };
25+
if ("draft" in r && typeof r.draft !== "boolean") {
26+
return "invalid_draft";
27+
}
28+
}
29+
return "invalid_shape";
30+
}
31+
32+
export function normalizeLabelerRule(rule: unknown): LabelerRule | null {
33+
if (Array.isArray(rule) && rule.every((x) => typeof x === "string")) {
34+
return rule as string[];
35+
}
36+
if (
37+
rule !== null &&
38+
typeof rule === "object" &&
39+
"globs" in rule &&
40+
Array.isArray((rule as { globs: unknown }).globs) &&
41+
(rule as { globs: unknown[] }).globs.every((x) => typeof x === "string")
42+
) {
43+
const r = rule as { globs: string[]; draft?: unknown };
44+
if ("draft" in r && typeof r.draft !== "boolean") {
45+
return null;
46+
}
47+
const out: { globs: string[]; draft?: boolean } = { globs: r.globs };
48+
if (typeof r.draft === "boolean") {
49+
out.draft = r.draft;
50+
}
51+
return out;
52+
}
53+
return null;
54+
}
55+
56+
export function globsFromRule(rule: LabelerRule): string[] {
57+
return Array.isArray(rule) ? rule : rule.globs;
58+
}
59+
60+
export function draftConstraintAllowsLabel(
61+
rule: LabelerRule,
62+
isDraft: boolean
63+
): boolean {
64+
const draftOpt = Array.isArray(rule) ? undefined : rule.draft;
65+
if (draftOpt === undefined) {
66+
return true;
67+
}
68+
if (draftOpt === false) {
69+
return !isDraft;
70+
}
71+
return isDraft;
72+
}
73+
74+
export async function getLabelsFromLabelerConfig(
75+
context: Context,
76+
labelerConfigTracker: CachedLabelerConfigTracker,
77+
changed_files: string[],
78+
isDraft: boolean = false
79+
): Promise<string[]> {
80+
const config = await labelerConfigTracker.loadLabelsConfig(context);
81+
const labels: string[] = [];
82+
83+
for (const [label, rawRule] of Object.entries(config)) {
84+
const rule = normalizeLabelerRule(rawRule);
85+
if (rule === null) {
86+
const skipReason = labelerRuleSkipReason(rawRule);
87+
if (skipReason === "invalid_draft") {
88+
context.log(
89+
{
90+
label,
91+
rawRule,
92+
draft: (rawRule as { draft?: unknown }).draft,
93+
},
94+
"getLabelsFromLabelerConfig: invalid draft type (expected boolean), skipping"
95+
);
96+
} else {
97+
context.log(
98+
{ label, rawRule },
99+
"getLabelsFromLabelerConfig: unknown rule shape, skipping"
100+
);
101+
}
102+
continue;
103+
}
104+
if (!draftConstraintAllowsLabel(rule, isDraft)) {
105+
continue;
106+
}
107+
const globs = globsFromRule(rule);
108+
if (
109+
globs.some((glob: string) =>
110+
changed_files.some((file: string) => minimatch(file, glob))
111+
)
112+
) {
113+
labels.push(label);
114+
}
115+
}
116+
return labels;
117+
}
118+
119+
export function getDraftGatedLabelsToRemove(
120+
config: Record<string, unknown>,
121+
changed_files: string[],
122+
isDraft: boolean
123+
): string[] {
124+
const toRemove: string[] = [];
125+
for (const [label, rawRule] of Object.entries(config)) {
126+
const rule = normalizeLabelerRule(rawRule);
127+
if (rule === null || Array.isArray(rule) || rule.draft === undefined) {
128+
continue;
129+
}
130+
const globsMatch = globsFromRule(rule).some((glob) =>
131+
changed_files.some((file) => minimatch(file, glob))
132+
);
133+
if (globsMatch && !draftConstraintAllowsLabel(rule, isDraft)) {
134+
toRemove.push(label);
135+
}
136+
}
137+
return toRemove;
138+
}

0 commit comments

Comments
 (0)