From e71eae4b35a7bc707b9412b1d4515191539546df Mon Sep 17 00:00:00 2001 From: Dexterity104 Date: Fri, 26 Jun 2026 14:01:08 +0000 Subject: [PATCH] fix(scoring): anchor the draft title pattern to genuine markers --- src/scoring/pending-pr-scenarios.ts | 7 +++-- src/signals/contributor-open-pr-monitor.ts | 3 +- test/unit/contributor-open-pr-monitor.test.ts | 20 +++++++++++++ test/unit/pending-pr-scenarios.test.ts | 29 +++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/scoring/pending-pr-scenarios.ts b/src/scoring/pending-pr-scenarios.ts index 67368cf7cd..358f037f32 100644 --- a/src/scoring/pending-pr-scenarios.ts +++ b/src/scoring/pending-pr-scenarios.ts @@ -37,6 +37,10 @@ export type ContributorRepoOpenPrSignals = { const STALE_DAYS = 14; +// Real draft markers only — "[draft]", "Draft:", "Draft -"; the delimiter keeps "Drafting" and +// "draft-js" from matching. Trailing \s* lets the same pattern also strip the marker for dedup keys. +export const DRAFT_TITLE_PATTERN = /^(?:\[\s*draft\s*\]|draft(?:\s*:|\s+-))\s*/i; + export async function loadContributorRepoOpenPrSignalRecords( env: Env, repoFullName: string, @@ -221,8 +225,7 @@ export function applyPendingPrDetectionToScoreInput( function isDraftPullRequest(pr: PullRequestRecord): boolean { if (pr.isDraft) return true; - const title = pr.title.trim(); - if (/^\[?\s*draft\s*\]?/i.test(title) || /^draft:/i.test(title)) return true; + if (DRAFT_TITLE_PATTERN.test(pr.title.trim())) return true; return pr.labels.some((label) => label.toLowerCase() === "draft" || label.toLowerCase() === "wip"); } diff --git a/src/signals/contributor-open-pr-monitor.ts b/src/signals/contributor-open-pr-monitor.ts index 30769503b2..8989440f5d 100644 --- a/src/signals/contributor-open-pr-monitor.ts +++ b/src/signals/contributor-open-pr-monitor.ts @@ -3,6 +3,7 @@ import { sanitizePublicComment } from "../github/commands"; import { classifyOpenPullRequest, detectPendingPrScenario, + DRAFT_TITLE_PATTERN, loadContributorRepoOpenPrSignals, type ClassifiedOpenPullRequest, type PendingPrScenarioDetection, @@ -243,7 +244,7 @@ function duplicatePronePullNumbers(openPullRequests: PullRequestRecord[]): Set { expect(__contributorOpenPrMonitorInternals.duplicatePronePullNumbers([pr({ number: 42, labels: ["wip"] })]).has(42)).toBe(true); }); + it("strips only genuine draft markers when normalizing titles for dedup (regression)", () => { + const { duplicatePronePullNumbers } = __contributorOpenPrMonitorInternals; + + // "[draft] X" and "X" are the same work, so stripping the marker collapses them into one cluster. + const clustered = duplicatePronePullNumbers([ + pr({ number: 60, title: "[draft] fix parser bug" }), + pr({ number: 61, title: "fix parser bug" }), + ]); + expect(clustered.has(60)).toBe(true); + expect(clustered.has(61)).toBe(true); + + // A title that merely starts with the word "draft" keeps it, so it is not mistaken for "tooling". + const distinct = duplicatePronePullNumbers([ + pr({ number: 62, title: "Draft tooling" }), + pr({ number: 63, title: "tooling" }), + ]); + expect(distinct.has(62)).toBe(false); + expect(distinct.has(63)).toBe(false); + }); + it("covers monitor summaries, guidance, next steps, and file heuristics", () => { const { nextStepsForClassification, summarizeMonitor, buildMonitorGuidance, missingTestsFromFiles, priorityRank } = __contributorOpenPrMonitorInternals; diff --git a/test/unit/pending-pr-scenarios.test.ts b/test/unit/pending-pr-scenarios.test.ts index ea8248034d..5aea1593de 100644 --- a/test/unit/pending-pr-scenarios.test.ts +++ b/test/unit/pending-pr-scenarios.test.ts @@ -302,6 +302,35 @@ describe("pending PR scenario detection", () => { expect(withOverlapFlags.reasons.join(" ")).toMatch(/duplicate|test files/i); }); + it("does not treat titles that merely start with the letters 'draft' as drafts (regression)", () => { + // These all begin with "draft" but carry no real marker, so they must flow through to merge_ready + // instead of being dropped — otherwise pendingMergedPrCount is silently understated. The hyphenated + // names ("draft-js", "draft-mode") are the cases an unspaced `draft[-:]` boundary would mis-flag. + for (const title of ["Drafting a new feature", "Draftsman tool", "Drafted changes", "draft-js upgrade", "Draft-mode rendering rewrite"]) { + expect( + classifyOpenPullRequest({ + pr: pr({ number: 60, title }), + roleContext: outsideContributorRole, + reviews: [approvedReview(60)], + checks: [], + }).classification, + ).toBe("merge_ready"); + } + }); + + it("treats only genuine draft markers in the title as drafts", () => { + for (const title of ["[draft] spike", "[ draft ] spike", "Draft: spike", "draft : spike", "Draft - spike", "Draft -spike"]) { + expect( + classifyOpenPullRequest({ + pr: pr({ number: 61, title }), + roleContext: outsideContributorRole, + reviews: [approvedReview(61)], + checks: [], + }).classification, + ).toBe("draft"); + } + }); + it("recognizes draft heuristics and excludes pull numbers from detection", () => { expect( classifyOpenPullRequest({