[Bug]: `buildRepoOutcomePatterns` ignores the `recent_merged_pull_requests` table — merge-rate analysis runs almost without merged PRs, falsely flagging healthy repos as "high closure risk"

[galuis116]

Summary

buildRepoOutcomePatterns builds its analyzed PR set by iterating only args.pullRequests, and uses args.recentMergedPullRequests purely as an enrichment lookup for PRs that already appear in args.pullRequests:

// src/signals/engine.ts:1780-1783 — recent-merged is only a by-number lookup map
const mergedDetailByNumber = new Map<number, RecentMergedPullRequestRecord>();
for (const record of args.recentMergedPullRequests ?? []) {
  if (record.repoFullName.toLowerCase() === repoKey) mergedDetailByNumber.set(record.number, record);
}
...
// src/signals/engine.ts:1802-1805 — analyzed iterates ONLY args.pullRequests
const analyzed: RepoOutcomePullRequest[] = args.pullRequests
  .filter((pr) => pr.repoFullName.toLowerCase() === repoKey)
  .map((pr) => {
    const mergedDetail = mergedDetailByNumber.get(pr.number);   // enrich only; never adds merged PRs
    ...
  });

A merged PR that exists only in the recent_merged_pull_requests table (i.e. not also in pull_requests) is never added to analyzed, so it is invisible to totals.merged, outsideContributorMergeRate, the per-dimension merge rates, and every success/risk pattern.

Why merged PRs are absent from args.pullRequests

The production loader reads the two from separate tables:

// src/services/repo-outcome-patterns.ts:74-75
listPullRequests(env, fullName),               // -> pull_requests table
listRecentMergedPullRequests(env, fullName),   // -> recent_merged_pull_requests table

And the backfill writes merged PRs to the recent-merged table only, while the pull_requests table gets open PRs (and reconciled-closed shells):

• The open_pull_requests segment fetches state=open and upserts into pull_requests, then markUnseenOpenPullRequestsClosed flips any vanished open PR to state:"closed" with no mergedAt (src/github/backfill.ts:884-897).
• The recent_merged_pull_requests segment fetches merged PRs and writes them via upsertRecentMergedPullRequest to the separate recent_merged_pull_requests table (src/github/backfill.ts:915-925). The monolithic path does the same split (backfill.ts:1321-1326).

So for any repo populated by backfill (every registered repo), the historical merged PRs live only in recent_merged_pull_requests. listPullRequests (src/db/repositories.ts) returns only the pull_requests rows. Net: args.pullRequests contains open + reconciled-closed (+ a few PRs merged live via webhook while installed) — but not the merged history.

Downstream effect

outsideContributorMergeRate and every outcome dimension are computed from analyzed (which excludes the merged history):

// src/signals/engine.ts:1843
const outsideContributorMergeRate = rate(outsideDecided.filter((pr) => pr.merged).length, outsideDecided.length);
// :1854 — all 7 dimensions iterate only outsideDecided
for (const pr of outsideDecided) { ...path/label/size/linked_issue/test_evidence/author_role/review_churn... }

With the merged PRs missing, outsideContributorMergeRate collapses toward 0, which:

• Suppresses the "Outside contributors merge well here" success pattern (fires only when >= REPO_OUTCOME_MERGE_WELL_RATE = 0.7, engine.ts:1888).
• Falsely fires the "Outside contributor PRs rarely merge here" / high-closure-risk risk pattern (fires when <= REPO_OUTCOME_CLOSURE_RISK_RATE = 0.34, engine.ts:1896-1903).
• Compounds via mislabeling: a PR that actually merged but was only ever seen as open and then reconciled to closed (no mergedAt) is counted as closed_unmerged (engine.ts:1807), pushing the merge rate even lower.

RepoOutcomePatterns feeds buildContributorStrategy, buildRepoFitRecommendation, reward-risk, and the decision pack / maintainer guidance — so a repo that merges outside contributions well is reported as high-closure-risk, steering miners away from exactly the repos where their PRs would land, and corrupting the path/label/size/test-evidence dimension breakdowns (which never see merged history at all).

Failure mode (concrete example)

A registered repo has 5 cached open PRs (in pull_requests) and 200 historically merged outside-contributor PRs (in recent_merged_pull_requests). Of the 5 open, 3 were later reconciled to closed (no mergedAt).

• Current: analyzed ≈ the 5 pull_requests rows; outsideDecided = the 3 reconciled-closed (counted closed_unmerged); outsideContributorMergeRate = 0/3 = 0 → 0 <= 0.34 → emits "Outside contributor PRs rarely merge here … expect a high closure rate" with confidence medium.
• Correct: the 200 merged PRs are counted → outsideContributorMergeRate ≈ 0.97 → emits "Outside contributors merge well here".

The reported conclusion is the exact opposite of reality.

Steps to reproduce

1. For a repo, populate pull_requests with a few open/closed PRs and recent_merged_pull_requests with many merged PRs (the normal backfill outcome).
2. Call computeRepoOutcomePatterns(env, repoFullName) (or buildRepoOutcomePatterns directly with pullRequests = open/closed only and recentMergedPullRequests = the merged set).
3. Inspect totals.merged and riskPatterns: totals.merged ≈ 0 and a false "Outside contributor PRs rarely merge here" risk pattern, instead of the merged PRs being counted.

Expected behavior

The analyzed PR set includes the merged PRs from recent_merged_pull_requests, so totals.merged, outsideContributorMergeRate, the dimension merge rates, and the success/risk patterns reflect real merge history.

Actual behavior

analyzed iterates only args.pullRequests; recentMergedPullRequests is used only to enrich already-present PRs, so the merged history is dropped from every count and rate, producing false high-closure-risk patterns and blind dimension breakdowns.

Suggested fix

• Build a unified decided-PR set: normalize each recent_merged_pull_requests record into a RepoOutcomePullRequest (bucket: "merged", decided: true, merged: true, linked from linkedIssues, labels, filePaths from changedFiles), de-duplicate against args.pullRequests by number (prefer the merged record), and feed the merged set through the same analyzed → decided → dimension-grouping logic.
• RecentMergedPullRequestRecord (src/types.ts) has no authorAssociation, so derive maintainerLane / authorRole for merged-only PRs from payload.author_association when present, else default to outside/external (the conservative choice for the outside-contributor merge rate). Note changedFiles carries no per-file additions/deletions, so changedLineCount for merged-only PRs is unavailable (use 0 or size-bucket by file count).
• Fix the reconciliation mislabel so a reconciled-closed PR that actually merged (present in recent_merged_pull_requests) is counted as merged, not closed_unmerged.
• Add fail-on-revert coverage: buildRepoOutcomePatterns with pullRequests = a few closed PRs and recentMergedPullRequests = many merged outside-contributor PRs must report a high outsideContributorMergeRate / "merge well" pattern (and totals.merged > 0), not a high-closure-risk pattern. Existing tests only pass merged PRs that also appear in pullRequests, so they never exercise the recent-merged-only path.

[dosubot]

I can confirm this bug report is accurate. The code clearly shows the issue:

1. recentMergedPullRequests is only a lookup map — at lines 1779–1782, the function builds a Map<number, RecentMergedPullRequestRecord> but never iterates it to add PRs to the analyzed set.

2. analyzed iterates only args.pullRequests — at lines 1801–1827, the analyzed array is constructed exclusively from args.pullRequests.filter(...). A merged PR that exists only in recent_merged_pull_requests is invisible.

3. The backfill writes to separate tables — the service loads them independently: listPullRequests (up to 500 from pull_requests) and listRecentMergedPullRequests (up to 200 from recent_merged_pull_requests). The backfill puts merged PRs in the latter table and open PRs in the former, so the merged history is never in args.pullRequests for backfill-populated repos.

4. No test coverage for this path — existing tests either overlap the same PR numbers in both collections or omit recentMergedPullRequests entirely. No test verifies behavior when a merged PR exists exclusively in recentMergedPullRequests.

The suggested fix approach (building a unified decided-PR set by normalizing recentMergedPullRequests records into RepoOutcomePullRequest entries, de-duplicating by number, and fixing the reconciliation mislabel) looks sound. The main consideration is that RecentMergedPullRequestRecord lacks authorAssociation, so deriving maintainerLane/authorRole will need a fallback (defaulting to outside/external is conservative and correct for the outside-contributor merge rate). Similarly, changedFiles lacks per-file line counts, so changedLineCount would need to fall back to file-count-based sizing.

Written by @dosu, approved by JSONbored

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.

[JSONbored]

Issue is once again valid, closing due to repeated issue closure gaming (attempting to gain multipliers after repeated warnings).