perf(github): skip the files fetch for already-hydrated merged PRs (#1941)

[JSONbored]

Summary

The recent-merged backfill segment (and the monolithic path) fetched GET /pulls/{n}/files for every merged PR on every sync — an N+1 REST fan-out. But a merged PR is immutable: its changed-file list never changes once stored, so re-fetching it every 6-hour full sync is pure waste.

This skips the per-PR /files fetch for any merged PR already hydrated: one listRecentMergedPullRequests read per batch builds the set of PRs that already have stored files, and those PRs get only a cheap metadata re-upsert (the upsert already preserves stored files when passed an empty list — repositories.ts:3144). Both identical N+1 sites now go through a shared hydrateMergedPullRequestFiles helper.

Advances #1941 (rec #9 from the #1936 audit).

Scope

• Conventional Commit title.
• Focused, backend-only change (one file + a test), DRYs two duplicate loops into one helper.
• Follows CONTRIBUTING.md; no site//CNAME.
• Linked issue (advances feat(github): collapse status reads with GraphQL rollups #1941).

Validation

• git diff --check
• npm run typecheck
• npm run test:coverage — added a regression test that pre-stores a merged PR with files, runs the recent-merged segment, and asserts zero /pulls/{n}/files fetches while the metadata is still refreshed and the stored files preserved (it fails without the skip). The not-already-stored (fetch) branch stays covered by the existing "hydrates merged PR changed files" test; the changed region is 100% covered (statements + branches).
• npm run test:ci · all 134 backfill tests pass (the refactor into the shared helper is behavior-preserving)
• npm audit --audit-level=moderate

If any required check was skipped, explain why:

• No migration / OpenAPI / cf-typegen: single-file backend change, no schema/API/binding change.

Safety

• No secrets/wallets/hotkeys/trust-scores/etc.
• No public GitHub text change.
• No auth/CORS/session change.
• No API/OpenAPI/MCP change. No UI change.

Notes

• Correctness: only the expensive /files fetch is skipped; the merged PR's metadata (title/labels/merged_at) is still re-upserted each pass, and skipping only happens when files are already stored (a first-time or files-less record still fetches). Merged PRs are terminal, so their file list cannot change after storage.
• Also removed a provably-dead .catch(() => []) on fetchPullRequestFiles (it never throws — githubPaginatedList catches per-page and returns []/undefined), which the original inline sites carried.

[loopover-orb]

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-01 05:14:03 UTC}

2 files · 1 AI reviewer · no blockers · readiness 86/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

• Touches a guarded path — held for manual review

Review summary
The change correctly removes duplicate per-PR file hydration loops and centralizes the skip logic for merged PRs that already have stored changed files. The preservation path is consistent with the documented upsert behavior, and the added regression test exercises the recent-merged segment’s most important no-refetch case while still refreshing metadata. The remaining risk is coverage/maintainability around the monolithic backfill caller and the helper’s reliance on the list API returning the full hydrated set.

Nits — 6 non-blocking

• nit: src/github/backfill.ts:1257 relies on listRecentMergedPullRequests(env, repoFullName) returning every stored merged PR; if that helper has a default limit, older hydrated PRs will silently fall back to the expensive /files path.
• nit: test/unit/backfill.test.ts:1023 covers the recent_merged_pull_requests segment skip path, but the monolithic backfill path changed at src/github/backfill.ts:1718 has no visible regression assertion for the same already-hydrated skip behavior.
• nit: src/github/backfill.ts:1267 removed the previous local .catch(() => []) safety net; if fetchPullRequestFiles is intended to be non-throwing, keeping that contract documented at the helper boundary or in its tests would make this refactor easier to maintain.
• Confirm listRecentMergedPullRequests has no default pagination limit for this use, or pass an explicit limit/all-records option at src/github/backfill.ts:1257 so the skip set is complete.
• Add a monolithic backfill regression beside the new segment test to assert src/github/backfill.ts:1718 also avoids /pulls/{n}/files for an already-hydrated merged PR.
• Touches a guarded path — held for manual review — A maintainer must review and merge this change.

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewer
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (size label size:M; no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 68 registered-repo PR(s), 58 merged, 209 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 68 PR(s), 209 issue(s).
Gate result	⚠️ Not blocking	Advisory; not blocking this PR.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository registration is not available in the local Gittensory cache.
• Public profile languages: not available
• Official Gittensor activity: 68 PR(s), 209 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• Triage stale or unlinked PRs.
• No action.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.62%. Comparing base (ff13838) to head (5720582).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1985   +/-   ##
=======================================
  Coverage   95.61%   95.62%           
=======================================
  Files         218      218           
  Lines       24306    24308    +2     
  Branches     8817     8818    +1     
=======================================
+ Hits        23241    23244    +3     
  Misses        436      436           
+ Partials      629      628    -1     

Files with missing lines	Coverage Δ
src/github/backfill.ts	95.74% <100.00%> (+0.11%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.