[Bug]: Repo sync-status rollup excludes the `recent_merged_pull_requests` segment, so an unfinished merged-history crawl is marked `success` and then skipped as "fresh"

[galuis116]

Summary

refreshRepoSyncStateFromSegments computes a repo's overall sync status from a required segment list that omits the recent_merged_pull_requests segment, even though it loads and reads that segment:

// src/github/backfill.ts:1344-1362
const [previous, totals, metadata, labels, openIssues, openPullRequests, recentMerged, files, reviews, checks] = await Promise.all([ ... ]);
const required = [metadata, labels, openIssues, openPullRequests, files, reviews, checks].filter(Boolean) as RepoSyncSegmentRecord[];   // <- recentMerged missing
const waiting = required.some((segment) => segment.status === "waiting_rate_limit" || segment.status === "rate_limited");
const running = required.some((segment) => segment.status === "running" || segment.status === "refreshing");
const errored = required.some((segment) => segment.status === "error");
const incomplete = required.some((segment) => segment.status !== "complete" && segment.status !== "not_modified");
const status: RepoSyncStateRecord["status"] = waiting ? "rate_limited" : errored ? "error" : running ? "running" : incomplete ? "partial" : "success";
const warnings = [...new Set(required.flatMap((segment) => segment.warnings))];
const completedAt = running || waiting ? previous?.lastCompletedAt : nowIso();

recentMerged is destructured (:1351) and used for recentMergedPullRequestsCount (:1373) and mergedPullRequestsSyncedAt (:1377), but it is not in required, so its running / waiting_rate_limit / error / non-terminal status — and its warnings — are ignored when deriving the repo-level status.

Why this is wrong

recent_merged_pull_requests is a first-class tracked backfill segment everywhere else. The parallel monolithic backfill path's summarizeSegments derives partial from all segments (recent-merged included):

// src/github/backfill.ts  (summarizeSegments — monolithic path)
partial: segments.some((segment) => segment.status !== "complete" && segment.status !== "not_modified") || warnings.length > 0,

So the two backfill code paths now disagree on whether the merged-history crawl's completeness affects sync status. The segment-rollup path is the outlier that drops it.

Failure mode

A registered repo is backfilled via the segment pipeline; all four base segments plus PR-detail segments are queued, and refreshRepoSyncStateFromSegments runs after each segment completes. Suppose open_issues, open_pull_requests, pull_request_files/reviews/checks, labels, and metadata all reach complete, but recent_merged_pull_requests is still running (or waiting_rate_limit, or error, or otherwise non-terminal — the merged-history crawl is the largest and most rate-limit-prone):

• Current: required excludes recent-merged, so waiting/running/errored/incomplete are all false → status = "success", and completedAt = nowIso() → lastCompletedAt = now.
• Correct: status should be rate_limited / error / running / partial, reflecting the unfinished merged-history crawl.

Then the freshness check skips the repo on the next run:

// src/github/backfill.ts:345-364
const freshSuccess = (syncState.status === "success" || syncState.status === "partial" || syncState.status === "capped") && Number.isFinite(ageMs) && ageMs < FRESH_SYNC_MS;
...
if (freshSuccess || recentError) {
  return { ..., status: "skipped", ... };   // skips backfillRepository for FRESH_SYNC_MS (~6h)
}

So the repo is treated as freshly synced and skipped for the freshness window, leaving its recent-merged PR history silently stale/undercounted — and it also produces an internal inconsistency: status === "success" while mergedPullRequestsSyncedAt (set only when recent-merged is complete/sampled, :1377) remains stale.

Downstream impact

recentMergedPullRequestsCount and the merged-PR history feed buildRepoOutcomePatterns (merge-rate / outcome dimensions), buildCollisionReport (recent-merged collisions), and contributor merge-history. Marking an incomplete merged-history crawl as success and skipping its refresh leaves those signals computed on partial data — wrong merge-rate/closure-risk outcomes (compounding the recent-merged data issues already tracked in #312 and #352, but via a different mechanism: premature success + freshness-skip, not empty file lists).

Steps to reproduce

1. Run the segment backfill for a repo where recent_merged_pull_requests does not reach a terminal state (e.g. it returns running/waiting_rate_limit/error) while the other segments complete.
2. Read the repo sync state (getRepoSyncState).
3. Observe status: "success" with a fresh lastCompletedAt, despite the merged-history segment being unfinished — and backfillRegisteredRepositories then reports the repo as skipped for the freshness window.

Expected behavior

The repo-level sync status and warnings reflect the recent_merged_pull_requests segment's state: a running / waiting_rate_limit / error / non-terminal merged-history crawl yields running / rate_limited / error / partial (not success), so the freshness-skip does not prematurely mark it fresh.

Actual behavior

The merged-history segment is excluded from the status/warnings derivation, so an unfinished crawl is reported as success with a fresh lastCompletedAt, and the repo is then skipped as fresh.

Suggested fix

• Include recent_merged_pull_requests in the segment set that feeds waiting / running / errored / incomplete and warnings. Adding it to required alone is not correct: recent-merged legitimately ends in the "sampled" terminal state (it uses progressiveHistory: true; status = "sampled" at backfill.ts:1164, and :1377 already treats sampled as synced), which the incomplete = status !== "complete" && status !== "not_modified" predicate would wrongly flag as incomplete, making the repo perpetually partial.
• So introduce sampled-aware terminal handling — e.g. a small helper/predicate that treats complete, not_modified, and (for the history segment) sampled as terminal — and apply it across incomplete (and the non-terminal checks) for the full segment set including recent-merged. This aligns the segment-rollup path with summarizeSegments and keeps status consistent with mergedPullRequestsSyncedAt.
• Add fail-on-revert coverage: when every base/detail segment is complete but recent_merged_pull_requests is running (and again error / waiting_rate_limit), the rolled-up status is running / error / rate_limited (not success) and lastCompletedAt is not advanced; and when recent-merged is sampled with the rest complete, status is success (sampled is terminal, not partial).

[dosubot]

Confirmed this bug — recentMerged is destructured from the Promise.all results but excluded from the required array that feeds the status derivation [1]. The status logic at lines 1361–1365 therefore ignores recentMerged's running/waiting_rate_limit/error state entirely [2], which can mark the repo "success" prematurely and trigger the 6-hour freshness-skip [3].

The inconsistency with summarizeSegments (used in the monolithic backfillRepository path) is also confirmed — that function evaluates all segments including recent_merged_pull_requests when computing partial [4].

The suggested fix direction (sampled-aware terminal predicate) makes sense. "sampled" is already treated as a valid terminal state for mergedPullRequestsSyncedAt at line 1377 [5], so adding recentMerged to required without adjusting the incomplete predicate would indeed regress repos into perpetual "partial". A small helper like:

const isTerminal = (s: RepoSyncSegmentRecord) =>
  s.status === "complete" || s.status === "not_modified" || s.status === "sampled";

applied to incomplete (and used by waiting/running/errored as their non-terminal counterpart) would close this without side effects.

This compounds the data gaps from #312 [6] and #352 [7] — those affect what data is stored for merged PRs, while this bug affects whether the crawl is even allowed to finish before the repo is declared fresh.

To reply, just mention @dosu.

---

Share context across your team and agents. Try Dosu.