From 57abcb9c43276e924c5d41cadbac5304e3d2c9ff Mon Sep 17 00:00:00 2001 From: aiyakitori <16633065+chiang-daniel@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:41:17 +0200 Subject: [PATCH 1/3] Remind about WIP-titled PRs weekly instead of daily A WIP-titled PR is one the author is still working on, so a daily "convert to draft, or drop WIP" nudge in #prs is noise (Slack thread, Oct 14: Daniel + Leonard agreed on weekly). Set the wip_title reminder threshold to 120 working hours (5 weekdays) in the team config. Only time spent in the wip_title state is affected: the threshold is read from the PR's current state on each sweep, and any state change resets the staleness clock, so a PR whose title drops "WIP" gets the normal 24-hour reminders from that point. A sweep test covers both. Co-Authored-By: Claude Fable 5.1 --- nathan.config.ts | 3 +++ specs/projects/nathan/functional_spec.md | 2 +- .../pr_management/reminders_sweep.test.ts | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/nathan.config.ts b/nathan.config.ts index 7a9ad77..ee1d9ec 100644 --- a/nathan.config.ts +++ b/nathan.config.ts @@ -25,6 +25,9 @@ export default defineConfig({ repos: ["Kiln-AI/Kiln", "Kiln-AI/kiln_server"], channel: "C0996APVD9R", // #prs triager: "chiang-daniel", + // WIP-titled PRs are reminded weekly (120 working hours = 5 weekdays), not daily: a daily + // "convert to draft" nudge is noise while the author is still working. + reminders: { thresholdHoursByState: { wip_title: 120 } }, }, }, diff --git a/specs/projects/nathan/functional_spec.md b/specs/projects/nathan/functional_spec.md index 5e99284..900c377 100644 --- a/specs/projects/nathan/functional_spec.md +++ b/specs/projects/nathan/functional_spec.md @@ -150,7 +150,7 @@ There are two equivalent entry points, and both end in the same state: reviewers ### 4.6 Stale reminders - **Staleness clock:** hours elapsed since the PR entered its current state and owner set. Weekend hours (Saturday and Sunday in the **owner's** Slack time zone) don't count, so tone doesn't escalate over a weekend nobody worked. Any state or owner change resets the clock and the escalation level. -- **Threshold:** 24 hours (1 day) for all states. The `urgent` modifier (label) shortens this to 4 hours. Thresholds are configurable per state. +- **Threshold:** 24 hours (1 day) for all states, except WIP-titled PRs (§4.2), which get 120 working hours (a week) so a PR someone is still working on isn't nudged daily. The `urgent` modifier (label) shortens this to 4 hours. Thresholds are configurable per state. - **Delivery:** an hourly sweep posts a threaded reply on the card, @-tagging each overdue owner. Reminders are sent as soon as they're due, at any time of day. Team policy is "send anytime, read when you're working", so there are no quiet hours. - **Repeat:** after the first reminder, the next is due after another full threshold interval, at an escalation level one higher. - **Tone escalation:** configurable message templates grouped by level (1, 2, 3, 4+). Each level has several variants, picked at random (avoiding the variant used last time on that PR) so it stays fresh. Level 1 is friendly, and later levels get progressively more pointed and humorous. The highest level repeats indefinitely. Every reminder states the next step and age. diff --git a/test/features/pr_management/reminders_sweep.test.ts b/test/features/pr_management/reminders_sweep.test.ts index 6cf9998..a3b9364 100644 --- a/test/features/pr_management/reminders_sweep.test.ts +++ b/test/features/pr_management/reminders_sweep.test.ts @@ -192,6 +192,32 @@ describe("stale reminders", () => { expect(threadReplies(h)).toHaveLength(2); }); + it("reminds about a WIP title weekly when configured, and daily again once the title loses WIP", async () => { + const h = prApp({ config: prConfig({ reminders: { thresholdHoursByState: { wip_title: 120 } } }) }); + await seenAt(h, "2026-10-05T14:00:00Z", { title: "WIP: Add the thing" }); + // A normal PR would be reminded after a day; a WIP one waits five working days (the weekend doesn't count). + await sweepAt(h, "2026-10-06T14:00:00Z"); + await sweepAt(h, "2026-10-09T14:00:00Z"); + expect(threadReplies(h)).toEqual([]); + await sweepAt(h, "2026-10-12T14:00:00Z"); + expect(threadReplies(h)).toHaveLength(1); + expect(threadReplies(h)[0]?.text).toContain( + '*Next step:* Convert to draft, or drop "WIP" from the title · waiting 1w', + ); + + // Dropping WIP changes the state, which resets the clock: the new state's 24-hour threshold applies. + h.github.upsert(aPR({ title: "Add the thing" })); + await sweepAt(h, "2026-10-12T15:00:00Z"); + await sweepAt(h, "2026-10-13T14:00:00Z"); + expect(threadReplies(h)).toHaveLength(1); + await sweepAt(h, "2026-10-13T15:00:00Z"); + expect(threadReplies(h)).toHaveLength(2); + // Back at level 1 (a different variant: a PR never gets the same line twice in a row). + const [tagged, facts] = threadReplies(h)[1]?.text.split("\n") ?? []; + expect(LEVEL_1.map((line) => `<@UALICE> — ${line}`)).toContain(tagged); + expect(facts).toContain("*Next step:* Request a reviewer · waiting 1d"); + }); + it("never reminds about drafts", async () => { const h = prApp(); await seenAt(h, "2026-10-05T14:00:00Z", { isDraft: true }); From 4f11c0f12f2094a47149d5e4cc3e207db67ccd5d Mon Sep 17 00:00:00 2001 From: aiyakitori <16633065+chiang-daniel@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:48:58 +0200 Subject: [PATCH 2/3] Tighten the WIP reminder comments Co-Authored-By: Claude Fable 5.1 --- nathan.config.ts | 3 +-- test/features/pr_management/reminders_sweep.test.ts | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/nathan.config.ts b/nathan.config.ts index ee1d9ec..5d62381 100644 --- a/nathan.config.ts +++ b/nathan.config.ts @@ -25,8 +25,7 @@ export default defineConfig({ repos: ["Kiln-AI/Kiln", "Kiln-AI/kiln_server"], channel: "C0996APVD9R", // #prs triager: "chiang-daniel", - // WIP-titled PRs are reminded weekly (120 working hours = 5 weekdays), not daily: a daily - // "convert to draft" nudge is noise while the author is still working. + // WIP-titled PRs are still being worked on: remind weekly (5 working days), not daily. reminders: { thresholdHoursByState: { wip_title: 120 } }, }, }, diff --git a/test/features/pr_management/reminders_sweep.test.ts b/test/features/pr_management/reminders_sweep.test.ts index a3b9364..ea67c6c 100644 --- a/test/features/pr_management/reminders_sweep.test.ts +++ b/test/features/pr_management/reminders_sweep.test.ts @@ -195,7 +195,6 @@ describe("stale reminders", () => { it("reminds about a WIP title weekly when configured, and daily again once the title loses WIP", async () => { const h = prApp({ config: prConfig({ reminders: { thresholdHoursByState: { wip_title: 120 } } }) }); await seenAt(h, "2026-10-05T14:00:00Z", { title: "WIP: Add the thing" }); - // A normal PR would be reminded after a day; a WIP one waits five working days (the weekend doesn't count). await sweepAt(h, "2026-10-06T14:00:00Z"); await sweepAt(h, "2026-10-09T14:00:00Z"); expect(threadReplies(h)).toEqual([]); @@ -205,14 +204,14 @@ describe("stale reminders", () => { '*Next step:* Convert to draft, or drop "WIP" from the title · waiting 1w', ); - // Dropping WIP changes the state, which resets the clock: the new state's 24-hour threshold applies. + // Dropping WIP changes the state, which resets the clock; the new state's 24-hour threshold applies. h.github.upsert(aPR({ title: "Add the thing" })); await sweepAt(h, "2026-10-12T15:00:00Z"); await sweepAt(h, "2026-10-13T14:00:00Z"); expect(threadReplies(h)).toHaveLength(1); await sweepAt(h, "2026-10-13T15:00:00Z"); expect(threadReplies(h)).toHaveLength(2); - // Back at level 1 (a different variant: a PR never gets the same line twice in a row). + // Level 1 again, in a fresh variant: a PR never gets the same line twice in a row. const [tagged, facts] = threadReplies(h)[1]?.text.split("\n") ?? []; expect(LEVEL_1.map((line) => `<@UALICE> — ${line}`)).toContain(tagged); expect(facts).toContain("*Next step:* Request a reviewer · waiting 1d"); From defae019038c64a251f7a36db234fbdbccd3e6c8 Mon Sep 17 00:00:00 2001 From: aiyakitori <16633065+chiang-daniel@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:07:50 +0200 Subject: [PATCH 3/3] Make the weekly WIP threshold Nathan's default, not a team override The rule is about what a WIP title means (the PR is still being worked on), so it belongs in the code's defaults rather than nathan.config.ts: thresholdHoursByState now defaults to { wip_title: 120 }, and a team's per-state overrides are merged on top so setting another state's threshold doesn't silently drop the WIP week. The spec and setup checklist describe the default; the sweep test runs on default config and asserts the exact reminders, like its neighbours. Co-Authored-By: Claude Fable 5.1 --- docs/setup.md | 4 +-- nathan.config.ts | 2 -- specs/projects/nathan/functional_spec.md | 2 +- src/features/pr_management/config.ts | 13 ++++++++-- test/features/pr_management/config.test.ts | 15 +++++++++-- .../pr_management/reminders_sweep.test.ts | 25 +++++++++---------- 6 files changed, 39 insertions(+), 22 deletions(-) diff --git a/docs/setup.md b/docs/setup.md index 128bf5e..3879e98 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -214,7 +214,7 @@ Before starting: steps 1–7 done for staging, `npm run verify:github -- staging **Scheduled work** - [ ] The hourly sweep runs at the top of the hour: cards' ages update, and any PR that changed while webhooks were off is corrected (try one with the Worker's GitHub webhook temporarily deactivated). -- [ ] Reminders: a PR that sits past its threshold (24 working hours, 4 with `urgent`) gets a reminder reply in its card's thread. To see one sooner, deploy staging with a lowered `reminders.thresholdHours` for a day. +- [ ] Reminders: a PR that sits past its threshold (24 working hours; 4 with `urgent`, 120 for a WIP title) gets a reminder reply in its card's thread. To see one sooner, deploy staging with a lowered `reminders.thresholdHours` for a day. - [ ] Drafts older than 14 days produce `[dry-run → DM …]` nudges. - [ ] The daily report posts at 09:30 ET on a weekday, with the Trends and People sections on Monday, and its **Request PR** button opens the form. - [ ] `/nathan-staging prs` and the Home tab show "Waiting on you" and "Your open PRs" for someone in `users`, and instructions for someone who isn't. @@ -247,7 +247,7 @@ Production launches in two moves: first deployed with `pr_management` off, to ch - [ ] **The first sweep backfills cards** at the next top of the hour: one card for every open, non-draft PR with reviewers requested, including PRs the old workflow already posted. Expect that burst in `#prs`. - [ ] **Old drafts get one DM each:** drafts older than 14 days are nudged on the first sweep (once, however old). -- [ ] Reminders don't start yet: staleness is measured from when Nathan first saw each PR, so the first reminders come one threshold (24 working hours) after launch. +- [ ] Reminders don't start yet: staleness is measured from when Nathan first saw each PR, so the first reminders come one threshold (24 working hours; a week for WIP titles) after launch. - [ ] The first daily report, the next weekday at 09:30 ET, has no previous report to compare against, so it shows no change in the open count. The Needs attention section fills in once reminders start. - [ ] Request one real review through the form and watch its card and handoffs. - [ ] The admin channel stays quiet, and `nathan-jobs-production-dlq` stays empty. diff --git a/nathan.config.ts b/nathan.config.ts index 5d62381..7a9ad77 100644 --- a/nathan.config.ts +++ b/nathan.config.ts @@ -25,8 +25,6 @@ export default defineConfig({ repos: ["Kiln-AI/Kiln", "Kiln-AI/kiln_server"], channel: "C0996APVD9R", // #prs triager: "chiang-daniel", - // WIP-titled PRs are still being worked on: remind weekly (5 working days), not daily. - reminders: { thresholdHoursByState: { wip_title: 120 } }, }, }, diff --git a/specs/projects/nathan/functional_spec.md b/specs/projects/nathan/functional_spec.md index 900c377..92a84cf 100644 --- a/specs/projects/nathan/functional_spec.md +++ b/specs/projects/nathan/functional_spec.md @@ -150,7 +150,7 @@ There are two equivalent entry points, and both end in the same state: reviewers ### 4.6 Stale reminders - **Staleness clock:** hours elapsed since the PR entered its current state and owner set. Weekend hours (Saturday and Sunday in the **owner's** Slack time zone) don't count, so tone doesn't escalate over a weekend nobody worked. Any state or owner change resets the clock and the escalation level. -- **Threshold:** 24 hours (1 day) for all states, except WIP-titled PRs (§4.2), which get 120 working hours (a week) so a PR someone is still working on isn't nudged daily. The `urgent` modifier (label) shortens this to 4 hours. Thresholds are configurable per state. +- **Threshold:** 24 hours (1 day) for all states except `wip_title`, which gets 120 hours (a working week): a WIP-titled PR is still being worked on, so a daily nudge would be noise. The `urgent` modifier (label) shortens this to 4 hours. Thresholds are configurable per state. - **Delivery:** an hourly sweep posts a threaded reply on the card, @-tagging each overdue owner. Reminders are sent as soon as they're due, at any time of day. Team policy is "send anytime, read when you're working", so there are no quiet hours. - **Repeat:** after the first reminder, the next is due after another full threshold interval, at an escalation level one higher. - **Tone escalation:** configurable message templates grouped by level (1, 2, 3, 4+). Each level has several variants, picked at random (avoiding the variant used last time on that PR) so it stays fresh. Level 1 is friendly, and later levels get progressively more pointed and humorous. The highest level repeats indefinitely. Every reminder states the next step and age. diff --git a/src/features/pr_management/config.ts b/src/features/pr_management/config.ts index 56c115d..7b313ec 100644 --- a/src/features/pr_management/config.ts +++ b/src/features/pr_management/config.ts @@ -25,6 +25,12 @@ export function isReminded(state: PRStatusState): state is RemindedState { return (REMINDED_STATES as readonly PRStatusState[]).includes(state); } +/** + * A WIP-titled PR is still being worked on, so its author is reminded weekly (5 working days) + * rather than daily. + */ +export const DEFAULT_THRESHOLD_HOURS_BY_STATE: Partial> = { wip_title: 120 }; + /** * Reminder lines by escalation level: levels 1, 2, 3, then 4 and up (the last group repeats). * Each is mrkdwn, posted after the owner tags and followed by the next step and age. @@ -57,8 +63,11 @@ const positiveHours = z.number().positive(); const remindersSchema = z.strictObject({ /** Working hours (weekends excluded) a PR may sit in one state before its owners are reminded. */ thresholdHours: positiveHours.default(24), - /** Overrides `thresholdHours` for particular states. */ - thresholdHoursByState: z.partialRecord(z.enum(REMINDED_STATES), positiveHours).default({}), + /** Overrides `thresholdHours` for particular states, on top of DEFAULT_THRESHOLD_HOURS_BY_STATE. */ + thresholdHoursByState: z + .partialRecord(z.enum(REMINDED_STATES), positiveHours) + .default({}) + .transform((byState) => ({ ...DEFAULT_THRESHOLD_HOURS_BY_STATE, ...byState })), /** The threshold for PRs requested with the `urgent` modifier (never longer than the state's own). */ urgentThresholdHours: positiveHours.default(4), /** Reminder lines grouped by level; see DEFAULT_REMINDER_TEMPLATES. */ diff --git a/test/features/pr_management/config.test.ts b/test/features/pr_management/config.test.ts index 3dee168..5b707f1 100644 --- a/test/features/pr_management/config.test.ts +++ b/test/features/pr_management/config.test.ts @@ -6,6 +6,7 @@ import { prManagement } from "../../../src/features/pr_management"; import { DEFAULT_BOT_AUTHORS, DEFAULT_REMINDER_TEMPLATES, + DEFAULT_THRESHOLD_HOURS_BY_STATE, DEFAULT_WIP_TITLE_PATTERN, } from "../../../src/features/pr_management/config"; import { prApp, prConfig } from "../../helpers/pr"; @@ -23,7 +24,7 @@ describe("pr_management config", () => { wipTitlePattern: DEFAULT_WIP_TITLE_PATTERN, reminders: { thresholdHours: 24, - thresholdHoursByState: {}, + thresholdHoursByState: DEFAULT_THRESHOLD_HOURS_BY_STATE, urgentThresholdHours: 4, templates: DEFAULT_REMINDER_TEMPLATES, }, @@ -45,11 +46,21 @@ describe("pr_management config", () => { drafts: { nudgeEveryDays: 3 }, }), ).toMatchObject({ - reminders: { thresholdHours: 24, thresholdHoursByState: { awaiting_review: 8 }, templates: [["Ping"]] }, + reminders: { + thresholdHours: 24, + thresholdHoursByState: { wip_title: 120, awaiting_review: 8 }, + templates: [["Ping"]], + }, drafts: { nudgeAfterDays: 14, nudgeEveryDays: 3 }, }); }); + it("lets a per-state threshold replace the WIP default", () => { + expect(load({ reminders: { thresholdHoursByState: { wip_title: 24 } } })).toMatchObject({ + reminders: { thresholdHoursByState: { wip_title: 24 } }, + }); + }); + it("ships at least two variants for each of levels 1, 2, 3 and 4+", () => { expect(DEFAULT_REMINDER_TEMPLATES).toHaveLength(4); for (const group of DEFAULT_REMINDER_TEMPLATES) expect(group.length).toBeGreaterThanOrEqual(2); diff --git a/test/features/pr_management/reminders_sweep.test.ts b/test/features/pr_management/reminders_sweep.test.ts index ea67c6c..ea33952 100644 --- a/test/features/pr_management/reminders_sweep.test.ts +++ b/test/features/pr_management/reminders_sweep.test.ts @@ -192,29 +192,28 @@ describe("stale reminders", () => { expect(threadReplies(h)).toHaveLength(2); }); - it("reminds about a WIP title weekly when configured, and daily again once the title loses WIP", async () => { - const h = prApp({ config: prConfig({ reminders: { thresholdHoursByState: { wip_title: 120 } } }) }); + it("reminds about a WIP title after a working week, then on the default schedule once the title changes", async () => { + const h = prApp(); await seenAt(h, "2026-10-05T14:00:00Z", { title: "WIP: Add the thing" }); await sweepAt(h, "2026-10-06T14:00:00Z"); - await sweepAt(h, "2026-10-09T14:00:00Z"); + await sweepAt(h, "2026-10-12T13:00:00Z"); expect(threadReplies(h)).toEqual([]); + await sweepAt(h, "2026-10-12T14:00:00Z"); - expect(threadReplies(h)).toHaveLength(1); - expect(threadReplies(h)[0]?.text).toContain( - '*Next step:* Convert to draft, or drop "WIP" from the title · waiting 1w', - ); + expect(threadReplies(h).map((post) => post.text)).toEqual([ + `<@UALICE> — ${LEVEL_1[0]}\n*Next step:* Convert to draft, or drop "WIP" from the title · waiting 1w · opened 1w 3d ago`, + ]); - // Dropping WIP changes the state, which resets the clock; the new state's 24-hour threshold applies. + // Dropping WIP changes the state, which restarts the clock under the default threshold. h.github.upsert(aPR({ title: "Add the thing" })); await sweepAt(h, "2026-10-12T15:00:00Z"); await sweepAt(h, "2026-10-13T14:00:00Z"); expect(threadReplies(h)).toHaveLength(1); + await sweepAt(h, "2026-10-13T15:00:00Z"); - expect(threadReplies(h)).toHaveLength(2); - // Level 1 again, in a fresh variant: a PR never gets the same line twice in a row. - const [tagged, facts] = threadReplies(h)[1]?.text.split("\n") ?? []; - expect(LEVEL_1.map((line) => `<@UALICE> — ${line}`)).toContain(tagged); - expect(facts).toContain("*Next step:* Request a reviewer · waiting 1d"); + expect(threadReplies(h).at(-1)?.text).toBe( + `<@UALICE> — ${LEVEL_1[1]}\n*Next step:* Request a reviewer · waiting 1d · opened 1w 5d ago`, + ); }); it("never reminds about drafts", async () => {