Skip to content

[Bug]: Segment-pipeline backfill never fetches merged-PR changed files (and clobbers any that exist), so recent_merged_pull_requests.changedFiles is always empty in production #352

Description

@philluiz2323

Summary

The production backfill pipeline persists every recent-merged PR with an empty changedFiles list. backfillRecentMergedSegment (the per-segment job path used for registered repos) hard-codes [] for the file list:

// src/github/backfill.ts:1076-1080  (segment pipeline — production path)
const merged = payloads.filter((pr) => Boolean(pr.merged_at));
await mapWithConcurrency(merged, 8, async (pr) => {
  await upsertRecentMergedPullRequest(env, toRecentMergedPullRequest(repo.fullName, pr, []));   // <- always []
});

The only path that fetches the real file list is the monolithic backfillRepository path, which is the legacy/whole-repo path, not the segment jobs that run in production:

// src/github/backfill.ts:1478-1479  (monolithic path — fetches files)
const changedFiles = await fetchPullRequestFiles(env, repo.fullName, pr.number, token, warnings).catch(() => []);
await upsertRecentMergedPullRequest(env, toRecentMergedPullRequest(repo.fullName, pr, changedFiles));

toRecentMergedPullRequest maps the files straight through (changedFiles: files.map((file) => file.filename), backfill.ts:1825-1836), so when the segment path passes [], the stored list is empty.

Worse, the upsert unconditionally overwrites the stored files, so a segment backfill wipes any list a previous monolithic run had populated:

// src/db/repositories.ts — upsertRecentMergedPullRequest
.onConflictDoUpdate({
  target: ...,
  set: { ..., changedFilesJson: jsonString(pr.changedFiles), ... },   // [] clobbers existing files
})

And there is no detail-sync that hydrates merged-PR files afterward: the backfill-pr-details job (backfillOpenPullRequestDetails, backfill.ts:473-497) operates on listOpenPullRequests only — it never touches recent_merged_pull_requests.

Net: in production, recent_merged_pull_requests.changedFiles is empty for every merged PR.

Why the segment path is the one that matters

Registered repos are backfilled by queued backfill-repo-segment jobs (BackfillSegmentName includes "recent_merged_pull_requests", backfill.ts:103,403), routed to backfillRecentMergedSegment (backfill.ts:458). The monolithic backfillRepository path is the legacy/all-at-once path. So the path that actually populates production data is the one that stores [].

Downstream effect

Consumers that read merged-PR file paths get nothing:

  • Repo outcome dimensions. buildRepoOutcomePatterns derives a merged PR's file paths from mergedDetail?.changedFiles (src/signals/engine.ts:1841). With it empty, the path, size, and test_evidence dimensions are blind for every merged PR — exactly the merged history those dimensions are meant to learn from. (This compounds the recent-merged-in-analysis work: even once merged PRs are counted, their file-based dimensions see nothing.)
  • Collision / duplicate-overlap. Recent-merged items feed the collision text used for overlap detection (changedFiles: pr.changedFiles, engine.ts:3421; collisionItemText joins changedFiles, engine.ts:3478). With empty files, a new PR can never be matched to a recently-merged PR by overlapping changed files.

Failure mode (concrete example)

A registered repo is backfilled by the segment pipeline. A merged PR #42 touched src/auth/session.ts and src/auth/session.test.ts.

  • Current: recent_merged_pull_requests row for #42 has changedFiles: []. In buildRepoOutcomePatterns, #42 contributes nothing to the path (src/auth/), size, or test_evidence (with_tests) dimensions; collision overlap can't match a new PR touching the same files.
  • Correct: #42 has changedFiles: ["src/auth/session.ts", "src/auth/session.test.ts"], so it counts toward the src/auth/ path dimension, a size bucket, and with_tests, and is available for file-overlap collision detection.

Steps to reproduce

  1. Run the segment backfill for a repo with merged PRs (backfill-repo-segment with segment recent_merged_pull_requests, the production path).
  2. Read the persisted recent_merged_pull_requests rows (e.g. listRecentMergedPullRequests).
  3. Observe every row has changedFiles: [], regardless of how many files the PR actually changed.
  4. (Clobber:) run the monolithic path first to populate files, then a segment backfill — the files are reset to [].

Expected behavior

The segment backfill populates each recent-merged PR's changedFiles (fetched via fetchPullRequestFiles, like the monolithic path), and the upsert preserves an existing non-empty file list rather than overwriting it with an empty one.

Actual behavior

The segment path stores changedFiles: [], the upsert clobbers any existing files with [], and no detail-sync hydrates merged-PR files — so the field is always empty in production.

Suggested fix

  • Hydrate merged-PR files in the segment pipeline. Either:
    • fetch files inside the recent_merged_pull_requests persistPage (mirroring the monolithic path's fetchPullRequestFiles, keeping the existing mapWithConcurrency and rate-limit handling), or
    • add a dedicated merged-PR detail-sync step analogous to backfill-pr-details but iterating listRecentMergedPullRequests, so file fetching is paginated/rate-limit-aware and re-runnable.
  • Make upsertRecentMergedPullRequest not clobber a populated changedFiles with an empty array (coalesce: on conflict, keep the existing changedFilesJson when the incoming list is empty), so a files-less segment pass cannot erase a populated one.
  • Add fail-on-revert coverage: a segment backfill of a merged PR persists its real changedFiles; and an empty-file upsert over an existing row preserves the previously stored files.

Metadata

Metadata

Assignees

No one assigned

    Labels

    slopAI slop and/or attempts to game additional points via manipulation or alt profiles.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions