Skip to content

Commit 43f72b3

Browse files
snomiaoclaudeCopilot
authored
feat(coreping): add time duration tracking for PR statuses (#122)
* feat(coreping): add time duration tracking for PR statuses - Add ms package for human-readable time duration formatting - Display "for X time" duration in status messages showing how long PRs have been in current state - Improve timeline event processing with better committed_at tracking - Refactor PR_STATUS determination logic for better maintainability - Update personalLabels config structure to support label-to-slack-user mapping - Add logging for skipped PRs with no status change - Clean up whitespace and code formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Update app/tasks/coreping/coreping.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(coreping.ts): update forDuration function to handle Date type for improved flexibility in time calculations * docs(coreping.ts): add comment for personalLabels to clarify its purpose refactor(coreping.ts): rename pull_request parameter to pr for consistency and clarity * Update app/tasks/coreping/coreping.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update app/tasks/coreping/coreping.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent ae1942e commit 43f72b3

3 files changed

Lines changed: 132 additions & 101 deletions

File tree

‎app/tasks/coreping/coreping.ts‎

Lines changed: 128 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#!/usr/bin/env bun --watch
2-
32
import { confirm } from "@/lib/utils";
43
import { tsmatch } from "@/packages/mongodb-pipeline-ts/Task";
54
import { db } from "@/src/db";
@@ -18,6 +17,7 @@ import DIE from "@snomiao/die";
1817
import chalk from "chalk";
1918
import { compareBy } from "comparing";
2019
import isCI from "is-ci";
20+
import ms from "ms";
2121
import sflow, { pageFlow } from "sflow";
2222
import { P } from "ts-pattern";
2323
import type { UnionToIntersection } from "type-fest";
@@ -58,9 +58,11 @@ export const coreReviewTrackerConfig = {
5858
],
5959
// labels
6060
labels: ["Core", "Core-Important", "CoreImportant"],
61-
personalLabels: {
62-
notifyJK: "notify:jk",
63-
},
61+
// personalLabels: label to slack user mapping
62+
personalLabels: [
63+
{ label: "notify:jk", slackUser: "@jk" },
64+
{ label: "notify:sno", slackUser: "snomiao" },
65+
],
6466
minReminderInterval: "24h", // edit-existed-slack-message < this-interval < send-another-slack-message
6567
slackChannelName: "develop", // develop channel, without notification
6668

@@ -73,6 +75,8 @@ const cfg = coreReviewTrackerConfig;
7375

7476
export const LABELS = cfg.labels;
7577

78+
type ReviewStatus = Awaited<ReturnType<typeof determinePullRequestReviewStatus>>["status"];
79+
7680
type ComfyCorePRs = {
7781
url: string;
7882
title: string;
@@ -84,12 +88,14 @@ type ComfyCorePRs = {
8488
author?: string; // pr author login
8589

8690
// review status
87-
status: Awaited<ReturnType<typeof determinePullRequestReviewStatus>> | "UNRELATED";
88-
lastStatus?: Awaited<ReturnType<typeof determinePullRequestReviewStatus>> | "UNRELATED"; // for diff status and then send ping message
91+
status: ReviewStatus;
92+
statusAt?: Date;
93+
/** @deprecated use pr.title to compose status message */
94+
statusMsg?: string; // status message used to fill summary
95+
lastStatus?: ReviewStatus; // for diff status and then send ping message
8996

9097
// send a ping when status changed to pingable status
9198
isPingNeeded?: boolean;
92-
statusMsg?: string; // status message used to fill summary
9399
lastPingMessage?: null | {
94100
url: string;
95101
text: string; // ping message
@@ -153,7 +159,7 @@ if (import.meta.main) {
153159
}
154160
}
155161

156-
function reviewStatusExplained(status: Awaited<ReturnType<typeof determinePullRequestReviewStatus>> | "UNRELATED") {
162+
function reviewStatusExplained(status: ReviewStatus) {
157163
return tsmatch(status)
158164
.with("DRAFT", () => "The PR is in draft mode, not ready for review yet.")
159165
.with("MERGED", () => "The PR has been merged.")
@@ -180,79 +186,95 @@ function reviewStatusExplained(status: Awaited<ReturnType<typeof determinePullRe
180186
* 6. responded: A Latest change is responds by the PR author
181187
* 7. open: is ready for review, but no review/response yet
182188
*/
183-
async function determinePullRequestReviewStatus(pull_request: GH["pull-request-simple"] | GH["pull-request"] | string) {
184-
const pr =
185-
typeof pull_request !== "string"
186-
? pull_request
187-
: await ghc.pulls.get({ ...parsePullUrl(pull_request) }).then((e) => e.data);
189+
async function determinePullRequestReviewStatus(
190+
pr: GH["pull-request-simple"] | GH["pull-request"],
191+
{
192+
isUnrelated,
193+
}: {
194+
isUnrelated?: (pr: GH["pull-request-simple"] | GH["pull-request"]) => boolean;
195+
},
196+
) {
188197
return await tsmatch(pr)
189-
.with({ merged_at: P.string }, () => "MERGED" as const)
190-
.with({ state: "closed" }, () => "CLOSED" as const)
191-
.with({ draft: true }, () => "DRAFT" as const)
192-
.otherwise(
193-
async () => (await getTimelineReviewStatuses()).filter((e) => e.PR_STATUS).at(-1)?.PR_STATUS || ("OPEN" as const),
194-
);
195-
196-
async function getTimelineReviewStatuses() {
197-
const timeline = await ghPaged(ghc.issues.listEventsForTimeline)({
198-
...parseIssueUrl(pr.html_url),
199-
}).toArray();
200-
201-
const reviewers = timeline
202-
.map((e) =>
203-
tsmatch(e)
204-
.with(
205-
{
206-
event: "review_requested",
207-
requested_reviewer: { login: P.select() },
208-
},
209-
(e) => e,
210-
)
211-
.otherwise(() => null),
212-
)
213-
.filter(Boolean) as string[];
214-
215-
const timeline_statuses = timeline
216-
.map((e) => e as UnionToIntersection<typeof e>)
217-
.map((e) => ({
218-
...e,
219-
url: undefined,
220-
issue_url: undefined,
221-
id: undefined,
222-
node_id: undefined,
223-
performed_via_github_app: undefined,
224-
actor: e.actor?.login?.replace(/^/, "@"),
225-
user: e.user?.login?.replace(/^/, "@"),
226-
author: e.author?.name?.replace(/^/, ""),
227-
committer: e.committer?.name?.replace(/^/, "@"),
228-
tree: undefined,
229-
parents: undefined,
230-
verification: undefined,
231-
_links: undefined,
232-
sha: undefined,
233-
review_requester: e.review_requester?.login?.replace(/^/, "@"),
234-
requested_reviewer: e.requested_reviewer?.login?.replace(/^/, "@"),
235-
label: e.label?.name,
236-
237-
reactions: e.reactions?.total_count,
238-
body: e.body?.replace(/\s+/g, " ").replace(/(?<=.{30})[\s\S]+/g, (e) => `...[${e.length} more chars]`),
239-
message: e.message?.replace(/\s+/g, " ").replace(/(?<=.{30})[\s\S]+/g, (e) => `...[${e.length} more chars]`),
240-
241-
PR_STATUS: tsmatch(e)
242-
.with({ event: "committed" }, () => "COMMITTED" as const)
243-
.with({ event: "reviewed" }, () => "REVIEWED" as const)
244-
.with({ event: "review_requested" }, () => "REVIEW_REQUESTED" as const)
245-
.with({ event: "commented" }, (e) =>
246-
reviewers.includes(e.user.login)
247-
? ("REVIEWER_COMMENTED" as const)
248-
: e.user.login === pr.user?.login
249-
? ("AUTHOR_COMMENTED" as const)
250-
: null,
251-
)
252-
.otherwise(() => null),
253-
}));
254-
return timeline_statuses;
255-
}
198+
.when(
199+
(pr) => isUnrelated?.(pr),
200+
() => ({ status: "UNRELATED" as const, statusAt: new Date(pr.updated_at!) }),
201+
)
202+
.with({ merged_at: P.string }, () => ({ status: "MERGED" as const, statusAt: new Date(pr.merged_at!) }))
203+
.with({ closed_at: P.string }, () => ({ status: "CLOSED" as const, statusAt: new Date(pr.closed_at!) }))
204+
.with({ draft: true }, () => ({ status: "DRAFT" as const, statusAt: new Date(pr.created_at) }))
205+
.otherwise(async () => {
206+
const latestEvent = (await getTimelineReviewStatuses(pr)).flatMap((e) => (e.PR_STATUS ? [e] : [])).at(-1);
207+
if (!latestEvent?.PR_STATUS) return { statusAt: new Date(pr.created_at), status: "OPEN" as const };
208+
const latestEventAt = latestEvent.committed_at || latestEvent.submitted_at || latestEvent.created_at;
209+
210+
if (!latestEventAt) throw new Error(`Failed to determine statusAt: no timestamp found in latest event for PR ${pr.html_url}`);
211+
212+
return { statusAt: new Date(latestEventAt), status: latestEvent.PR_STATUS };
213+
});
214+
}
215+
216+
async function getTimelineReviewStatuses(pr: GH["pull-request-simple"] | GH["pull-request"]) {
217+
const timeline = await ghPaged(ghc.issues.listEventsForTimeline)({ ...parseIssueUrl(pr.html_url) }).toArray();
218+
219+
const reviewers = timeline
220+
.map((e) =>
221+
tsmatch(e)
222+
.with(
223+
{
224+
event: "review_requested",
225+
requested_reviewer: { login: P.select() },
226+
},
227+
(e) => e,
228+
)
229+
.otherwise(() => null),
230+
)
231+
.filter(Boolean) as string[];
232+
233+
const timeline_statuses = timeline
234+
.map((e) => e as UnionToIntersection<typeof e>)
235+
// determine PR_STATUS, committed_at
236+
.map((e) => ({
237+
...e,
238+
PR_STATUS: tsmatch(e)
239+
.with({ event: "committed" }, () => "COMMITTED" as const)
240+
.with({ event: "reviewed" }, () => "REVIEWED" as const)
241+
.with({ event: "review_requested" }, () => "REVIEW_REQUESTED" as const)
242+
.with({ event: "commented" }, (e) =>
243+
reviewers.includes(e.user.login)
244+
? ("REVIEWER_COMMENTED" as const)
245+
: e.user.login === pr.user?.login
246+
? ("AUTHOR_COMMENTED" as const)
247+
: null,
248+
)
249+
.otherwise(() => null),
250+
committed_at: e.committer?.date,
251+
}))
252+
// sanitize output for debug
253+
.map((e) => ({
254+
...e,
255+
url: undefined,
256+
issue_url: undefined,
257+
id: undefined,
258+
node_id: undefined,
259+
performed_via_github_app: undefined,
260+
actor: e.actor?.login?.replace(/^/, "@"),
261+
user: e.user?.login?.replace(/^/, "@"),
262+
author: e.author?.name?.replace(/^/, ""),
263+
committer: e.committer?.name?.replace(/^/, "@"),
264+
tree: undefined,
265+
parents: undefined,
266+
verification: undefined,
267+
_links: undefined,
268+
sha: undefined,
269+
review_requester: e.review_requester?.login?.replace(/^/, "@"),
270+
requested_reviewer: e.requested_reviewer?.login?.replace(/^/, "@"),
271+
label: e.label?.name,
272+
273+
reactions: e.reactions?.total_count,
274+
body: e.body?.replace(/\s+/g, " ").replace(/(?<=.{30})[\s\S]+/g, (e) => `...[${e.length} more chars]`),
275+
message: e.message?.replace(/\s+/g, " ").replace(/(?<=.{30})[\s\S]+/g, (e) => `...[${e.length} more chars]`),
276+
}));
277+
return timeline_statuses;
256278
}
257279

258280
async function runCorePingTaskFull() {
@@ -291,7 +313,7 @@ async function runCorePingTaskFull() {
291313
$in: ["AUTHOR_COMMENTED", "REVIEW_REQUESTED", "OPEN", "COMMITTED"],
292314
},
293315
})
294-
.sort({ created_at: 1 })
316+
.sort({ statusAt: 1, created_at: 1 })
295317
.toArray();
296318

297319
const allOpeningCorePRs = await ComfyCorePRs.find({
@@ -311,16 +333,22 @@ async function runCorePingTaskFull() {
311333

312334
// console.log("ready to send slack message to notify @comfy");
313335
// console.log(processedTasks);
336+
337+
const forDuration = (at?: number | Date) => {
338+
if (!at) return "";
339+
const diff = Date.now() - (at instanceof Date ? at.getTime() : at);
340+
return "for " + ms(diff, { long: true });
341+
};
314342
const reviewMessage = !pendingReviewCorePRs.length
315343
? `Congratulations! All Core/Important PRs are reviewed! 🎉🎉🎉`
316344
: `Hey <@comfy>, Here's x${pendingReviewCorePRs.length} Core/Important PRs waiting your feedback!
317-
- ${pendingReviewCorePRs.map((pr) => pr.statusMsg || `<${pr.url}|${pr.title}> ${pr.labels}`).join("\n- ")}`;
345+
- ${pendingReviewCorePRs.map((pr) => `@${pr.author}: <${pr.url}|${pr.title}> (${pr.labels}) is ${pr.status} ${forDuration(pr.statusAt)}`).join("\n- ")}`;
318346
const keepInMindMessage =
319347
remainingOpeningCorePRs.length > 0
320348
? `\n\nAdditionally, there ${remainingOpeningCorePRs.length === 1 ? "is" : "are"} ${remainingOpeningCorePRs.length} other open Core/Important ${remainingOpeningCorePRs.length === 1 ? "PR" : "PRs"} that ${remainingOpeningCorePRs.length === 1 ? "is" : "are"} pending for author's change/update, lets wait for them.
321349
- ${remainingOpeningCorePRs
322350
.toSorted(compareBy((e) => e.created_at))
323-
.map((pr) => `@${pr.author}: <${pr.url}|${pr.title}> is ${pr.status}`)
351+
.map((pr) => `@${pr.author}: <${pr.url}|${pr.title}> is ${pr.status} ${forDuration(pr.statusAt)}`)
324352
.join("\n- ")}`
325353
: "";
326354
const tail = `\n\nSent from <https://github.com/Comfy-Org/Comfy-PR/blob/main/app/tasks/coreping/coreping.ts|CorePing.ts> by <@snomiao>`;
@@ -422,16 +450,15 @@ async function processPullRequestCorePingTask(
422450

423451
// save status & lastStatus
424452

425-
const status = !task.labels.some((e) => LABELS.includes(e))
426-
? "UNRELATED"
427-
: await determinePullRequestReviewStatus(pr);
453+
const { status, statusAt } = await determinePullRequestReviewStatus(pr, {
454+
isUnrelated: (pr) => !task.labels.some((e) => LABELS.includes(e)),
455+
});
456+
// update lastStatus if status changed
428457
const statusChanged = task.status !== status;
429458
if (statusChanged) {
430-
task = await saveTask({
431-
url: pr.html_url,
432-
status,
433-
lastStatus: task.status,
434-
});
459+
task = await saveTask({ url: pr.html_url, status, statusAt, lastStatus: task.status });
460+
} else {
461+
task = await saveTask({ url: pr.html_url, status, statusAt });
435462
}
436463

437464
// determine whether to ping in slack
@@ -458,30 +485,30 @@ async function processPullRequestCorePingTask(
458485
.otherwise(() => false);
459486

460487
console.log(
461-
`${i !== undefined ? i + 1 : ""} ${pr.html_url} # ${task.lastStatus || ""} => ${status} ${isPingNeeded ? chalk.red("PING") : ""}`.trim(),
488+
`${i !== undefined ? i + 1 : ""} ${pr.html_url} # ${task.lastStatus || ""} >> ${status} ${isPingNeeded ? chalk.red("PING") : ""} ${statusAt}`.trim(),
462489
);
463490

464491
if (task.lastStatus === status) {
465-
// console.log(`No status change for ${pr.html_url}, skipping`);
492+
console.log(`No status change for ${pr.html_url}, skipping`);
466493
return task;
467494
}
468495
// update status
469496
// task = await saveTask({
470497
// url: pr.html_url,
471498
// last_labeled_at: new Date(lastLabelEvent.created_at),
472499
// });
473-
const createdAt = new Date(pr.created_at);
474-
const now = new Date();
475-
const diff = now.getTime() - createdAt.getTime();
476-
const isFresh = diff <= 24 * 60 * 60 * 1000;
477-
const hours = Math.floor(diff / (60 * 60 * 1000));
478-
const sanitizedTitle = pr.title.replace(/\W+/g, " ").trim();
479-
const statusMsg = `@${pr.user?.login}: <${pr.html_url}|${sanitizedTitle}> is waiting for your feedback for more than ${hours} hours.`;
500+
// const now = new Date();
501+
// const diff = now.getTime() - (statusAt?.getTime() || now.getTime());
502+
// const isFresh = diff <= 24 * 60 * 60 * 1000;
503+
// const hours = Math.floor(diff / (60 * 60 * 1000));
504+
// const sanitizedTitle = pr.title.replace(/\W+/g, " ").trim();
505+
// const statusMsg = `@${pr.user?.login}: <${pr.html_url}|${sanitizedTitle}> is waiting for your feedback for ${hours} hours.`;
480506

481507
return await saveTask({
482508
url: html_url,
483509
status,
484-
statusMsg,
510+
statusAt,
511+
// statusMsg,
485512
isPingNeeded,
486513
...(!isPingNeeded ? { lastPingMessage: null } : {}),
487514
});

‎bun.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
"@trpc/react-query": "^11",
7474
"@trpc/server": "^11",
7575
"@types/ink": "^2.0.3",
76+
"@types/ms": "^2.1.0",
7677
"@types/parse-github-url": "^1.0.3",
7778
"@types/sha256": "^0.2.2",
7879
"async-sema": "^3.1.1",
@@ -113,6 +114,7 @@
113114
"minimist": "^1.2.8",
114115
"monaco-editor": "^0.50.0",
115116
"mongodb": "^7.0.0",
117+
"ms": "^2.1.3",
116118
"next-auth": "^5.0.0-beta.20",
117119
"nodemailer": "^6.9.14",
118120
"octokit": "^5.0.5",

0 commit comments

Comments
 (0)