From ac3e87e4155086f2facbe3d030ad52664fbaf671 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 20 Aug 2026 19:13:28 -0700 Subject: [PATCH 1/2] Add a bounded two-stage PR scan so portfolio can rank review and salvage opportunities without touching CLI. --- src/contracts/opportunities.ts | 28 ++ src/contracts/pr-scan.ts | 102 +++++++ src/core/pr-scan.ts | 494 +++++++++++++++++++++++++++++++++ test/pr-scan.test.ts | 249 +++++++++++++++++ 4 files changed, 873 insertions(+) create mode 100644 src/contracts/opportunities.ts create mode 100644 src/contracts/pr-scan.ts create mode 100644 src/core/pr-scan.ts create mode 100644 test/pr-scan.test.ts diff --git a/src/contracts/opportunities.ts b/src/contracts/opportunities.ts new file mode 100644 index 0000000..4bba866 --- /dev/null +++ b/src/contracts/opportunities.ts @@ -0,0 +1,28 @@ +import { z } from 'zod'; +import { RepoRefSchema } from './inputs.js'; + +/** + * Generic opportunity identity for portfolio/PR/eval surfaces. + * Do not replace the legacy issue-only TargetIdentitySchema. + */ +export const OpportunityTargetSchema = z.discriminatedUnion('kind', [ + z.object({ + kind: z.literal('issue'), + repo: RepoRefSchema, + issue_number: z.number().int().positive() + }).strict(), + z.object({ + kind: z.literal('pull_request'), + repo: RepoRefSchema, + pr_number: z.number().int().positive(), + linked_issue_number: z.number().int().positive().optional() + }).strict(), + z.object({ + kind: z.literal('eval_anomaly'), + repo: RepoRefSchema.optional(), + external_id: z.string().min(1), + source: z.string().min(1) + }).strict() +]); + +export type OpportunityTarget = z.infer; diff --git a/src/contracts/pr-scan.ts b/src/contracts/pr-scan.ts new file mode 100644 index 0000000..943c8e3 --- /dev/null +++ b/src/contracts/pr-scan.ts @@ -0,0 +1,102 @@ +import { z } from 'zod'; +import { OpportunityTargetSchema } from './opportunities.js'; + +export const PR_SCAN_VERSION = 1 as const; +export const PR_INVENTORY_LIMIT = 25 as const; +export const PR_ENRICH_LIMIT = 5 as const; + +export const PrScanFilterSchema = z.object({ + include_bots: z.boolean().default(false), + include_merged: z.boolean().default(false), + include_drafts: z.boolean().default(true), + include_generated: z.boolean().default(false), + stale_pr_days: z.number().int().positive().default(14), + inventory_limit: z.number().int().positive().max(PR_INVENTORY_LIMIT).default(PR_INVENTORY_LIMIT), + enrich_limit: z.number().int().positive().max(PR_ENRICH_LIMIT).default(PR_ENRICH_LIMIT) +}).strict(); + +export const PrHintModeSchema = z.enum(['REVIEW', 'WATCH', 'SALVAGE', 'PASS']); + +export const PrInventoryItemSchema = z.object({ + target: OpportunityTargetSchema, + repo: z.string().min(1), + number: z.number().int().positive(), + title: z.string(), + author: z.string().nullable(), + draft: z.boolean(), + state: z.enum(['open', 'closed']), + merged: z.boolean(), + created_at: z.string(), + updated_at: z.string(), + linked_issue_number: z.number().int().positive().optional(), + changed_files: z.number().int().nonnegative().optional(), + review_state: z.string().optional(), + ci_state: z.string().optional(), + cheap_rank: z.number().min(0).max(1), + filtered_reason: z.string().optional() +}).strict(); + +export const PrEnrichmentSchema = z.object({ + linked_issue_number: z.number().int().positive().optional(), + issue_open: z.boolean().optional(), + closes_issue: z.boolean().default(false), + review_states: z.array(z.string()).default([]), + maintainer_reviewed: z.boolean().default(false), + maintainer_positive_review: z.boolean().default(false), + requested_changes: z.boolean().default(false), + approved: z.boolean().default(false), + ci_state: z.enum(['success', 'failure', 'pending', 'unknown']).default('unknown'), + additions: z.number().int().nonnegative().optional(), + deletions: z.number().int().nonnegative().optional(), + changed_files: z.number().int().nonnegative().optional(), + touched_paths: z.array(z.string()).default([]), + has_tests: z.boolean().default(false), + competing_closers: z.number().int().nonnegative().default(0), + contention_gaps: z.array(z.string()).default([]), + stale: z.boolean().default(false), + stale_days: z.number().nonnegative().optional(), + maintainer_interest: z.boolean().default(false), + substantive: z.boolean().default(false), + credible_work: z.boolean().default(false), + healthy_active: z.boolean().default(false), + looks_like_bug: z.boolean().default(false), + cross_platform: z.boolean().default(false), + enormous_refactor: z.boolean().default(false) +}).strict(); + +export const PrOpportunitySchema = z.object({ + target: OpportunityTargetSchema, + inventory: PrInventoryItemSchema, + enriched: z.boolean(), + enrichment: PrEnrichmentSchema.optional(), + hint_mode: PrHintModeSchema, + hint_reasons: z.array(z.string()).default([]), + hard_constraints: z.array(z.string()).default([]), + salvage_facts: z.object({ + substantive_prior_attempt: z.boolean(), + stale_open_pr: z.boolean(), + maintainer_interest: z.boolean(), + credible_work_remains: z.boolean(), + healthy_active_closer: z.boolean(), + issue_open: z.boolean().optional() + }).optional() +}).strict(); + +export const PrScanResultSchema = z.object({ + pr_scan_version: z.literal(PR_SCAN_VERSION), + repo: z.string().min(1), + inventory_count: z.number().int().nonnegative(), + filtered_count: z.number().int().nonnegative(), + enriched_count: z.number().int().nonnegative(), + budget_truncated: z.boolean().default(false), + opportunities: z.array(PrOpportunitySchema).default([]), + checked: z.array(z.string()).default([]), + not_checked: z.array(z.string()).default([]) +}).strict(); + +export type PrScanFilter = z.infer; +export type PrInventoryItem = z.infer; +export type PrEnrichment = z.infer; +export type PrOpportunity = z.infer; +export type PrScanResult = z.infer; +export type PrHintMode = z.infer; diff --git a/src/core/pr-scan.ts b/src/core/pr-scan.ts new file mode 100644 index 0000000..3528539 --- /dev/null +++ b/src/core/pr-scan.ts @@ -0,0 +1,494 @@ +/** + * Bounded two-stage PR opportunity scan (GW-046). + * Cheap inventory → rank → enrich top N. Reuses contention/diff/gap helpers. + * Does not call CLI/MCP surfaces. + */ + +import { githubJson } from '../lib/github.js'; +import { fetchPullDiff, extractTouchedPaths } from '../lib/github-diff.js'; +import { createContentionBudget } from '../lib/contention-budget.js'; +import { getActiveRunBudget, noteCandidatesConsidered } from '../lib/run-budget.js'; +import { isAutomationAuthor } from './bots.js'; +import { closesIssue } from './linkage.js'; +import { contention } from './contention.js'; +import { + PR_ENRICH_LIMIT, + PR_INVENTORY_LIMIT, + PrScanFilterSchema, + PrScanResultSchema, + type PrEnrichment, + type PrHintMode, + type PrInventoryItem, + type PrOpportunity, + type PrScanFilter, + type PrScanResult +} from '../contracts/pr-scan.js'; + +export type PrScanInput = { + repo: string; + include_bots?: boolean; + include_merged?: boolean; + include_drafts?: boolean; + include_generated?: boolean; + stale_pr_days?: number; + inventory_limit?: number; + enrich_limit?: number; +}; + +type GithubPullListItem = { + number: number; + title: string; + body?: string | null; + state: string; + draft?: boolean; + merged_at?: string | null; + created_at: string; + updated_at: string; + html_url?: string; + user?: { login?: string } | null; + labels?: Array<{ name?: string }>; + head?: { sha?: string }; +}; + +type GithubPullDetail = GithubPullListItem & { + additions?: number; + deletions?: number; + changed_files?: number; + merged?: boolean; +}; + +type GithubReview = { + state?: string; + user?: { login?: string } | null; + author_association?: string; +}; + +type GithubIssueLite = { + state?: string; + number?: number; +}; + +type GithubCheckRuns = { + check_runs?: Array<{ conclusion?: string | null; status?: string }>; +}; + +const GENERATED_TITLE = /^\s*(\[bot(?:\s+pr)?\]|chore\(deps\)|bump\s+|deps:\s+)/i; +const BUG_HINT = /\b(bug|fix|crash|regression|error|fail(?:s|ed|ing)?)\b/i; +const PLATFORM_HINT = /\b(windows|linux|macos|osx|wsl|docker|container)\b/i; +const MAINTAINER_ASSOC = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']); +const CLOSING_ISSUE = /(?:fix(?:es)?|close[sd]?|resolve[sd]?)\s+#(\d+)\b/gi; + +function ageDays(iso: string, now = Date.now()): number { + const parsed = Date.parse(iso); + if (!Number.isFinite(parsed)) return 0; + return Math.max(0, Math.floor((now - parsed) / (24 * 60 * 60 * 1000))); +} + +function isTestPath(path: string): boolean { + return /(^|\/)(test|tests|__tests__|spec)(\/|$)/i.test(path) + || /\.(test|spec)\.[a-z]+$/i.test(path) + || /_test\.[a-z]+$/i.test(path); +} + +export function extractLinkedIssueNumber(title: string, body?: string | null): number | undefined { + const text = `${title}\n${body ?? ''}`; + const matches = [...text.matchAll(CLOSING_ISSUE)]; + const last = matches.at(-1); + return last ? Number(last[1]) : undefined; +} + +export function isGeneratedPr(title: string, labels: string[] = []): boolean { + if (GENERATED_TITLE.test(title)) return true; + return labels.some((label) => /dependencies|deps|automation/.test(label.toLowerCase())); +} + +export function filterInventoryReason( + item: { + author: string | null; + merged: boolean; + draft: boolean; + title: string; + labels?: string[]; + }, + filters: PrScanFilter +): string | undefined { + if (!filters.include_bots && isAutomationAuthor(item.author)) return 'automation_author'; + if (!filters.include_merged && item.merged) return 'closed_merged'; + if (!filters.include_drafts && item.draft) return 'draft'; + if (!filters.include_generated && isGeneratedPr(item.title, item.labels)) return 'generated'; + return undefined; +} + +export function cheapRankScore(item: { + draft: boolean; + merged: boolean; + state: 'open' | 'closed'; + updated_at: string; + linked_issue_number?: number; + title: string; + changed_files?: number; +}): number { + let score = 0.45; + if (item.state === 'open') score += 0.2; + if (item.linked_issue_number) score += 0.15; + if (!item.draft) score += 0.08; + if (BUG_HINT.test(item.title)) score += 0.07; + if (item.merged) score -= 0.3; + const days = ageDays(item.updated_at); + if (days <= 7) score += 0.08; + else if (days >= 60) score -= 0.08; + if (typeof item.changed_files === 'number' && item.changed_files > 80) score -= 0.1; + return Math.max(0, Math.min(1, score)); +} + +function enrichmentDefaults(input: Partial = {}): PrEnrichment { + return { + closes_issue: false, + review_states: [], + maintainer_reviewed: false, + maintainer_positive_review: false, + requested_changes: false, + approved: false, + ci_state: 'unknown', + touched_paths: [], + has_tests: false, + competing_closers: 0, + contention_gaps: [], + stale: false, + maintainer_interest: false, + substantive: false, + credible_work: false, + healthy_active: false, + looks_like_bug: false, + cross_platform: false, + enormous_refactor: false, + ...input + }; +} + +export function classifyPrHint(enrichmentInput: Partial, inventory: Pick): { + hint_mode: PrHintMode; + hint_reasons: string[]; + hard_constraints: string[]; +} { + const enrichment = enrichmentDefaults(enrichmentInput); + const reasons: string[] = []; + const constraints: string[] = []; + + if (inventory.merged) { + return { hint_mode: 'PASS', hint_reasons: ['Merged PRs are not contribution opportunities.'], hard_constraints: [] }; + } + + const salvageStrong = ( + (inventory.state === 'closed' && enrichment.substantive && enrichment.issue_open === true && enrichment.maintainer_positive_review) + || (inventory.state === 'open' && enrichment.stale && enrichment.maintainer_interest && enrichment.credible_work && (enrichment.requested_changes || enrichment.maintainer_positive_review)) + ); + const salvageWeakOnly = enrichment.stale && !enrichment.maintainer_interest && !enrichment.requested_changes && !enrichment.maintainer_positive_review; + + if (salvageStrong) { + constraints.push('coordinate_before_upstream_action', 'preserve_attribution', 'verify_current_main'); + reasons.push('Credible stalled implementation with maintainer engagement; salvage has a higher bar than age.'); + return { hint_mode: 'SALVAGE', hint_reasons: reasons, hard_constraints: constraints }; + } + + if (enrichment.healthy_active) { + reasons.push('Active PR looks healthy; watch until CI, review, or ownership changes.'); + return { hint_mode: 'WATCH', hint_reasons: reasons, hard_constraints: [] }; + } + + if (salvageWeakOnly || (enrichment.stale && inventory.draft && !enrichment.maintainer_interest)) { + reasons.push('Age or inactivity alone is not abandonment; inspect before salvage.'); + return { hint_mode: 'REVIEW', hint_reasons: reasons, hard_constraints: [] }; + } + + let reviewScore = 0.4; + if (enrichment.looks_like_bug) { reviewScore += 0.12; reasons.push('Looks like a real bug.'); } + if (!enrichment.maintainer_reviewed) { reviewScore += 0.1; reasons.push('No maintainer review yet.'); } + if (enrichment.competing_closers > 1) { reviewScore += 0.12; reasons.push('Competing closers need internal review.'); } + if (enrichment.ci_state === 'failure' || enrichment.ci_state === 'pending') { reviewScore += 0.08; reasons.push('CI is ambiguous or failing.'); } + if (!enrichment.has_tests) { reviewScore += 0.06; reasons.push('No test paths in the diff.'); } + if (enrichment.contention_gaps.length > 0) { reviewScore += 0.1; reasons.push('Contention gaps remain.'); } + if (enrichment.requested_changes) { reviewScore += 0.08; reasons.push('Requested changes are outstanding.'); } + if (enrichment.cross_platform) { reviewScore += 0.05; reasons.push('Cross-platform surface.'); } + + if (enrichment.healthy_active) reviewScore -= 0.2; + if (enrichment.enormous_refactor) { reviewScore -= 0.12; reasons.push('Enormous refactor; review value is lower.'); } + if (enrichment.approved && enrichment.ci_state === 'success') { reviewScore -= 0.15; reasons.push('Already approved and awaiting merge.'); } + + if (reviewScore < 0.35 && inventory.state === 'open' && !enrichment.stale) { + reasons.push('Open PR without a high-value review gap; watch for state change.'); + return { hint_mode: 'WATCH', hint_reasons: reasons, hard_constraints: [] }; + } + + reasons.push('Internal review/evidence is the justified contribution; do not post review comments automatically.'); + return { hint_mode: 'REVIEW', hint_reasons: reasons, hard_constraints: [] }; +} + +function toInventory(repo: string, pr: GithubPullListItem, filters: PrScanFilter): PrInventoryItem { + const author = pr.user?.login ?? null; + const labels = (pr.labels ?? []).map((label) => label.name ?? '').filter(Boolean); + const merged = Boolean(pr.merged_at); + const state: 'open' | 'closed' = pr.state === 'open' ? 'open' : 'closed'; + const linked = extractLinkedIssueNumber(pr.title, pr.body); + const item: Omit & { cheap_rank: number } = { + target: { + kind: 'pull_request' as const, + repo, + pr_number: pr.number, + ...(linked ? { linked_issue_number: linked } : {}) + }, + repo, + number: pr.number, + title: pr.title, + author, + draft: pr.draft === true, + state, + merged, + created_at: pr.created_at, + updated_at: pr.updated_at, + ...(linked ? { linked_issue_number: linked } : {}), + cheap_rank: 0 + }; + const filtered = filterInventoryReason({ ...item, labels }, filters); + return { + ...item, + cheap_rank: cheapRankScore(item), + ...(filtered ? { filtered_reason: filtered } : {}) + }; +} + +function looksLikeBug(title: string, labels: string[]): boolean { + return BUG_HINT.test(title) || labels.some((label) => /bug|regression/.test(label.toLowerCase())); +} + +async function enrichOne( + repo: string, + item: PrInventoryItem, + filters: PrScanFilter, + checked: string[], + notChecked: string[] +): Promise { + const detail = await githubJson(`/repos/${repo}/pulls/${item.number}`); + checked.push(`pull_detail#${item.number}`); + const labels = (detail.labels ?? []).map((label) => label.name ?? '').filter(Boolean); + const linked = item.linked_issue_number ?? extractLinkedIssueNumber(detail.title, detail.body); + const closes = linked ? closesIssue(`${detail.title}\n${detail.body ?? ''}`, linked) : false; + + let reviews: GithubReview[] = []; + try { + reviews = await githubJson(`/repos/${repo}/pulls/${item.number}/reviews`); + checked.push(`pull_reviews#${item.number}`); + } catch { + notChecked.push(`reviews unavailable for #${item.number}`); + } + + const maintainerReviews = reviews.filter((review) => MAINTAINER_ASSOC.has(review.author_association ?? '')); + const reviewStates = reviews.map((review) => review.state ?? 'UNKNOWN'); + const requestedChanges = reviewStates.includes('CHANGES_REQUESTED'); + const approved = reviewStates.includes('APPROVED'); + const maintainerPositive = maintainerReviews.some((review) => review.state === 'APPROVED' || review.state === 'COMMENTED'); + + let ciState: PrEnrichment['ci_state'] = 'unknown'; + const sha = detail.head?.sha; + if (sha) { + try { + const checks = await githubJson(`/repos/${repo}/commits/${sha}/check-runs`); + checked.push(`check_runs#${item.number}`); + const runs = checks.check_runs ?? []; + if (runs.some((run) => run.conclusion === 'failure' || run.conclusion === 'timed_out')) ciState = 'failure'; + else if (runs.some((run) => run.status === 'in_progress' || run.status === 'queued')) ciState = 'pending'; + else if (runs.length > 0 && runs.every((run) => run.conclusion === 'success' || run.conclusion === 'skipped' || run.conclusion === 'neutral')) { + ciState = 'success'; + } + } catch { + notChecked.push(`CI unavailable for #${item.number}`); + } + } + + let touched: string[] = []; + let hasTests = false; + try { + const budget = createContentionBudget(); + const diff = await fetchPullDiff(repo, item.number, budget); + touched = extractTouchedPaths(diff.text); + hasTests = touched.some((path) => isTestPath(path)); + checked.push(`pull_diff#${item.number}`); + } catch { + notChecked.push(`diff unavailable for #${item.number}`); + } + + let issueOpen: boolean | undefined; + let competing = 0; + const gaps: string[] = []; + if (linked) { + try { + const issue = await githubJson(`/repos/${repo}/issues/${linked}`); + issueOpen = issue.state !== 'closed'; + checked.push(`linked_issue#${linked}`); + } catch { + notChecked.push(`linked issue #${linked} unavailable`); + } + try { + const report = await contention({ repo, issue_number: linked, include_diffs: true, include_gaps: true }); + competing = report.contention.claims.filter((claim) => claim.state === 'open' && claim.closes_issue).length; + gaps.push(...report.contention.gaps.map((gap) => gap.kind)); + checked.push(`contention#${linked}`); + } catch { + notChecked.push(`contention unavailable for issue #${linked}`); + } + } + + const staleDays = ageDays(item.updated_at); + const stale = staleDays >= filters.stale_pr_days; + const additions = detail.additions ?? 0; + const deletions = detail.deletions ?? 0; + const changed = detail.changed_files ?? item.changed_files ?? touched.length; + const substantive = changed >= 3 || (additions + deletions) >= 20; + const credible = substantive && !item.draft && (maintainerPositive || requestedChanges || Boolean(linked)); + const healthy = item.state === 'open' + && !stale + && !requestedChanges + && maintainerReviews.length > 0 + && ciState === 'success' + && gaps.length === 0 + && competing <= 1; + + return { + linked_issue_number: linked, + issue_open: issueOpen, + closes_issue: closes, + review_states: reviewStates, + maintainer_reviewed: maintainerReviews.length > 0, + maintainer_positive_review: maintainerPositive, + requested_changes: requestedChanges, + approved, + ci_state: ciState, + additions: detail.additions, + deletions: detail.deletions, + changed_files: changed, + touched_paths: touched, + has_tests: hasTests, + competing_closers: competing, + contention_gaps: gaps, + stale, + stale_days: staleDays, + maintainer_interest: maintainerReviews.length > 0 || requestedChanges, + substantive, + credible_work: credible, + healthy_active: healthy, + looks_like_bug: looksLikeBug(item.title, labels), + cross_platform: PLATFORM_HINT.test(`${item.title}\n${detail.body ?? ''}`), + enormous_refactor: changed >= 80 || (additions + deletions) >= 1500 + }; +} + +/** Two-stage PR scan: cheap inventory, then enrich at most 5 ranked candidates. */ +export async function pr_scan(input: PrScanInput): Promise { + const filters = PrScanFilterSchema.parse({ + ...(input.include_bots !== undefined ? { include_bots: input.include_bots } : {}), + ...(input.include_merged !== undefined ? { include_merged: input.include_merged } : {}), + ...(input.include_drafts !== undefined ? { include_drafts: input.include_drafts } : {}), + ...(input.include_generated !== undefined ? { include_generated: input.include_generated } : {}), + ...(input.stale_pr_days !== undefined ? { stale_pr_days: input.stale_pr_days } : {}), + ...(input.inventory_limit !== undefined ? { inventory_limit: input.inventory_limit } : {}), + ...(input.enrich_limit !== undefined ? { enrich_limit: input.enrich_limit } : {}) + }); + const inventoryLimit = Math.min(filters.inventory_limit, PR_INVENTORY_LIMIT); + const enrichLimit = Math.min(filters.enrich_limit, PR_ENRICH_LIMIT); + const checked: string[] = [`listed pull requests for ${input.repo}`]; + const notChecked: string[] = [ + 'PR scan does not clone the repository.', + 'CLI/MCP wiring is deferred to the portfolio integration slice.' + ]; + + const listed = await githubJson( + `/repos/${input.repo}/pulls?state=all&sort=updated&direction=desc&per_page=${inventoryLimit}` + ); + noteCandidatesConsidered(listed.length); + + const inventory = listed.map((pr) => toInventory(input.repo, pr, filters)); + const kept = inventory.filter((item) => !item.filtered_reason); + const ranked = [...kept].sort((left, right) => right.cheap_rank - left.cheap_rank || right.number - left.number); + const toEnrich = ranked.slice(0, enrichLimit); + + const opportunities: PrOpportunity[] = []; + let budgetTruncated = false; + const activeBudget = getActiveRunBudget(); + + for (const item of ranked) { + const shouldEnrich = toEnrich.some((row) => row.number === item.number); + if (!shouldEnrich) { + const classified = classifyPrHint({ + closes_issue: Boolean(item.linked_issue_number), + stale: ageDays(item.updated_at) >= filters.stale_pr_days + }, item); + opportunities.push({ + target: item.target, + inventory: item, + enriched: false, + hint_mode: item.state === 'closed' && !item.merged ? 'REVIEW' : classified.hint_mode, + hint_reasons: item.state === 'closed' && !item.merged + ? ['Closed unmerged PR was not enriched; treat as REVIEW until salvage evidence exists.'] + : classified.hint_reasons, + hard_constraints: [] + }); + continue; + } + if (activeBudget?.counters.exhausted) { + budgetTruncated = true; + notChecked.push(`enrichment skipped for #${item.number} after run-budget exhaustion`); + opportunities.push({ + target: item.target, + inventory: item, + enriched: false, + hint_mode: 'REVIEW', + hint_reasons: ['Budget exhausted before enrichment; defaulting to REVIEW.'], + hard_constraints: [] + }); + continue; + } + try { + const enrichment = await enrichOne(input.repo, item, filters, checked, notChecked); + const classified = classifyPrHint(enrichment, item); + opportunities.push({ + target: item.target, + inventory: { ...item, changed_files: enrichment.changed_files, ci_state: enrichment.ci_state }, + enriched: true, + enrichment, + hint_mode: classified.hint_mode, + hint_reasons: classified.hint_reasons, + hard_constraints: classified.hard_constraints, + salvage_facts: { + substantive_prior_attempt: enrichment.substantive && item.state === 'closed' && !item.merged, + stale_open_pr: item.state === 'open' && enrichment.stale, + maintainer_interest: enrichment.maintainer_interest, + credible_work_remains: enrichment.credible_work, + healthy_active_closer: enrichment.healthy_active && enrichment.closes_issue, + issue_open: enrichment.issue_open + } + }); + } catch (error) { + notChecked.push(`enrichment failed for #${item.number}: ${error instanceof Error ? error.message : String(error)}`); + opportunities.push({ + target: item.target, + inventory: item, + enriched: false, + hint_mode: 'REVIEW', + hint_reasons: ['Enrichment failed; defaulting to REVIEW.'], + hard_constraints: [] + }); + } + } + + return PrScanResultSchema.parse({ + pr_scan_version: 1, + repo: input.repo, + inventory_count: listed.length, + filtered_count: inventory.length - kept.length, + enriched_count: opportunities.filter((item) => item.enriched).length, + budget_truncated: budgetTruncated, + opportunities, + checked, + not_checked: notChecked + }); +} diff --git a/test/pr-scan.test.ts b/test/pr-scan.test.ts new file mode 100644 index 0000000..a342b22 --- /dev/null +++ b/test/pr-scan.test.ts @@ -0,0 +1,249 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { OpportunityTargetSchema } from '../src/contracts/opportunities.js'; +import { PR_ENRICH_LIMIT, PR_INVENTORY_LIMIT, PrScanFilterSchema } from '../src/contracts/pr-scan.js'; +import { + cheapRankScore, + classifyPrHint, + extractLinkedIssueNumber, + filterInventoryReason, + isGeneratedPr +} from '../src/core/pr-scan.js'; + +const mocks = vi.hoisted(() => ({ + githubJson: vi.fn(), + contention: vi.fn(), + fetchPullDiff: vi.fn() +})); + +vi.mock('../src/lib/github.js', () => ({ + githubJson: mocks.githubJson, + githubToken: () => 'test-token' +})); + +vi.mock('../src/core/contention.js', () => ({ + contention: mocks.contention +})); + +vi.mock('../src/lib/github-diff.js', () => ({ + fetchPullDiff: mocks.fetchPullDiff, + extractTouchedPaths: (text: string) => [...text.matchAll(/^diff --git a\/(.+?) b\//gm)].map((match) => match[1] ?? '') +})); + +const { pr_scan } = await import('../src/core/pr-scan.js'); + +function pull(overrides: Record = {}) { + return { + number: 10, + title: 'Fix crash on Windows', + body: 'Fixes #88\n\nSteps to reproduce included.', + state: 'open', + draft: false, + merged_at: null, + created_at: '2026-08-01T00:00:00.000Z', + updated_at: '2026-08-18T00:00:00.000Z', + user: { login: 'alice' }, + labels: [{ name: 'bug' }], + head: { sha: 'abc123' }, + additions: 40, + deletions: 8, + changed_files: 4, + ...overrides + }; +} + +describe('OpportunityTargetSchema', () => { + it('accepts issue, pull_request, and eval_anomaly without changing TargetIdentity', () => { + expect(OpportunityTargetSchema.parse({ kind: 'issue', repo: 'o/r', issue_number: 1 }).kind).toBe('issue'); + expect(OpportunityTargetSchema.parse({ + kind: 'pull_request', repo: 'o/r', pr_number: 9, linked_issue_number: 3 + }).kind).toBe('pull_request'); + expect(OpportunityTargetSchema.parse({ + kind: 'eval_anomaly', external_id: 'eval-1', source: 'hermes-eval' + }).kind).toBe('eval_anomaly'); + }); +}); + +describe('cheap inventory filters and rank', () => { + const filters = PrScanFilterSchema.parse({}); + + it('filters bots, merged PRs, and generated dependency PRs', () => { + expect(filterInventoryReason({ author: 'dependabot[bot]', merged: false, draft: false, title: 'Bump left-pad' }, filters)).toBe('automation_author'); + expect(filterInventoryReason({ author: 'alice', merged: true, draft: false, title: 'Fix bug' }, filters)).toBe('closed_merged'); + expect(isGeneratedPr('chore(deps): bump lodash', ['dependencies'])).toBe(true); + expect(filterInventoryReason({ + author: 'alice', merged: false, draft: false, title: 'chore(deps): bump lodash', labels: ['dependencies'] + }, filters)).toBe('generated'); + expect(filterInventoryReason({ author: 'alice', merged: false, draft: false, title: 'Fix crash' }, filters)).toBeUndefined(); + }); + + it('ranks open linked bug PRs above stale drafts', () => { + const open = cheapRankScore({ + draft: false, merged: false, state: 'open', updated_at: '2026-08-18T00:00:00.000Z', + linked_issue_number: 8, title: 'Fix crash' + }); + const staleDraft = cheapRankScore({ + draft: true, merged: false, state: 'open', updated_at: '2026-01-01T00:00:00.000Z', + title: 'WIP refactor' + }); + expect(open).toBeGreaterThan(staleDraft); + }); + + it('extracts a cheap closing issue number', () => { + expect(extractLinkedIssueNumber('Fix login', 'Closes #41')).toBe(41); + expect(extractLinkedIssueNumber('Docs only', 'No closer here')).toBeUndefined(); + }); +}); + +describe('REVIEW / WATCH / SALVAGE heuristics', () => { + const open = { draft: false, state: 'open' as const, merged: false }; + const closed = { draft: false, state: 'closed' as const, merged: false }; + + it('does not classify age-only stale PRs as SALVAGE', () => { + const decision = classifyPrHint({ stale: true, stale_days: 40 }, open); + expect(decision.hint_mode).not.toBe('SALVAGE'); + expect(decision.hint_reasons.join(' ')).toMatch(/Age or inactivity alone is not abandonment/); + }); + + it('requires a high bar for SALVAGE and carries attribution constraints', () => { + const decision = classifyPrHint({ + stale: true, + maintainer_interest: true, + credible_work: true, + requested_changes: true, + substantive: true, + issue_open: true + }, open); + expect(decision.hint_mode).toBe('SALVAGE'); + expect(decision.hard_constraints).toEqual(expect.arrayContaining([ + 'coordinate_before_upstream_action', + 'preserve_attribution', + 'verify_current_main' + ])); + }); + + it('salvages a closed unmerged attempt only with maintainer-positive review and an open issue', () => { + const strong = classifyPrHint({ + substantive: true, + issue_open: true, + maintainer_positive_review: true + }, closed); + const weak = classifyPrHint({ + substantive: true, + issue_open: true + }, closed); + expect(strong.hint_mode).toBe('SALVAGE'); + expect(weak.hint_mode).not.toBe('SALVAGE'); + }); + + it('watches a healthy actively reviewed PR', () => { + const decision = classifyPrHint({ + healthy_active: true, + maintainer_reviewed: true, + ci_state: 'success', + closes_issue: true + }, open); + expect(decision.hint_mode).toBe('WATCH'); + }); + + it('increases REVIEW for competing closers, missing tests, and requested changes', () => { + const decision = classifyPrHint({ + looks_like_bug: true, + competing_closers: 2, + has_tests: false, + requested_changes: true, + contention_gaps: ['test_coverage'] + }, open); + expect(decision.hint_mode).toBe('REVIEW'); + expect(decision.hint_reasons.join(' ')).toMatch(/Competing closers|Requested changes|Contention gaps/); + }); +}); + +describe('pr_scan two-stage flow', () => { + beforeEach(() => { + mocks.githubJson.mockReset(); + mocks.contention.mockReset(); + mocks.fetchPullDiff.mockReset(); + mocks.fetchPullDiff.mockResolvedValue({ + text: 'diff --git a/src/app.ts b/src/app.ts\ndiff --git a/test/app.test.ts b/test/app.test.ts\n', + bytes: 40, + truncated: false, + additions: 12, + deletions: 2, + changed_files: 2 + }); + mocks.contention.mockResolvedValue({ + contention: { + claims: [ + { pr: 10, state: 'open', closes_issue: true }, + { pr: 11, state: 'open', closes_issue: true } + ], + gaps: [{ kind: 'test_coverage' }] + } + }); + }); + + it('lists, filters, ranks, and enriches only the top N', async () => { + const listed = [ + pull({ number: 1, title: 'chore(deps): bump left-pad', user: { login: 'dependabot[bot]' } }), + pull({ number: 2, title: 'Merged fix', state: 'closed', merged_at: '2026-08-01T00:00:00.000Z' }), + pull({ number: 10, title: 'Fix crash on Windows', body: 'Fixes #88' }), + pull({ number: 11, title: 'Docs tweak', body: 'n/a', labels: [] }), + pull({ number: 12, title: 'Closed attempt', state: 'closed', body: 'Fixes #90', merged_at: null }) + ]; + mocks.githubJson.mockImplementation(async (path: string) => { + if (path.includes('/pulls?')) return listed; + if (path.includes('/pulls/10/reviews') || path.includes('/pulls/11/reviews') || path.includes('/pulls/12/reviews')) { + return [{ state: 'CHANGES_REQUESTED', author_association: 'MEMBER' }]; + } + if (path.includes('/check-runs')) return { check_runs: [{ conclusion: 'failure', status: 'completed' }] }; + if (path.includes('/issues/')) return { state: 'open', number: 88 }; + if (path.includes('/pulls/')) { + const number = Number(path.split('/pulls/')[1]); + return listed.find((item) => item.number === number) ?? pull({ number }); + } + return []; + }); + + const result = await pr_scan({ repo: 'o/r', enrich_limit: 2, inventory_limit: 10 }); + expect(result.inventory_count).toBe(5); + expect(result.filtered_count).toBe(2); + expect(result.enriched_count).toBeLessThanOrEqual(2); + expect(result.opportunities.every((item) => item.target.kind === 'pull_request')).toBe(true); + const enriched = result.opportunities.filter((item) => item.enriched); + expect(enriched.length).toBeGreaterThan(0); + expect(enriched[0]?.enrichment?.competing_closers).toBe(2); + expect(mocks.contention).toHaveBeenCalled(); + }); + + it('respects conceptual bounds of 25 inventory and 5 enriched', () => { + expect(PR_INVENTORY_LIMIT).toBe(25); + expect(PR_ENRICH_LIMIT).toBe(5); + expect(PrScanFilterSchema.parse({ inventory_limit: 25, enrich_limit: 5 }).inventory_limit).toBe(25); + expect(() => PrScanFilterSchema.parse({ inventory_limit: 26 })).toThrow(); + }); + + it('does not salvage an unenriched stale draft', async () => { + mocks.githubJson.mockImplementation(async (path: string) => { + if (path.includes('/pulls?')) { + return [ + pull({ + number: 20, + title: 'WIP maybe later', + body: '', + draft: true, + updated_at: '2026-01-01T00:00:00.000Z', + labels: [] + }), + pull({ number: 21, title: 'Fix crash', body: 'Fixes #3' }) + ]; + } + if (path.includes('/reviews')) return []; + if (path.includes('/check-runs')) return { check_runs: [] }; + if (path.includes('/issues/')) return { state: 'open' }; + return pull({ number: 21 }); + }); + const result = await pr_scan({ repo: 'o/r', enrich_limit: 1, stale_pr_days: 14 }); + const stale = result.opportunities.find((item) => item.inventory.number === 20); + expect(stale?.hint_mode).not.toBe('SALVAGE'); + }); +}); From 30c39919fd9214398b2838cf443f1005c84b8ddd Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 20 Aug 2026 19:17:40 -0700 Subject: [PATCH 2/2] Require an open linked issue before classifying a stale PR as SALVAGE. --- src/core/pr-scan.ts | 4 +++- test/pr-scan.test.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/pr-scan.ts b/src/core/pr-scan.ts index 3528539..e8b60ce 100644 --- a/src/core/pr-scan.ts +++ b/src/core/pr-scan.ts @@ -181,7 +181,9 @@ export function classifyPrHint(enrichmentInput: Partial, inventory const salvageStrong = ( (inventory.state === 'closed' && enrichment.substantive && enrichment.issue_open === true && enrichment.maintainer_positive_review) - || (inventory.state === 'open' && enrichment.stale && enrichment.maintainer_interest && enrichment.credible_work && (enrichment.requested_changes || enrichment.maintainer_positive_review)) + || (inventory.state === 'open' && enrichment.stale && enrichment.maintainer_interest && enrichment.credible_work + && (enrichment.requested_changes || enrichment.maintainer_positive_review) + && enrichment.issue_open !== false) ); const salvageWeakOnly = enrichment.stale && !enrichment.maintainer_interest && !enrichment.requested_changes && !enrichment.maintainer_positive_review; diff --git a/test/pr-scan.test.ts b/test/pr-scan.test.ts index a342b22..30b7251 100644 --- a/test/pr-scan.test.ts +++ b/test/pr-scan.test.ts @@ -104,6 +104,17 @@ describe('REVIEW / WATCH / SALVAGE heuristics', () => { expect(decision.hint_reasons.join(' ')).toMatch(/Age or inactivity alone is not abandonment/); }); + it('does not salvage a stale open PR when the linked issue is closed', () => { + const decision = classifyPrHint({ + stale: true, + maintainer_interest: true, + credible_work: true, + requested_changes: true, + issue_open: false + }, open); + expect(decision.hint_mode).not.toBe('SALVAGE'); + }); + it('requires a high bar for SALVAGE and carries attribution constraints', () => { const decision = classifyPrHint({ stale: true,