fix(github): paginate GraphQL PR-detail fallback past 100 nodes#7493
Conversation
fetchPullRequestDetailsFromGraphQl's files/reviews connections requested
only first: 100 with no pageInfo, so a PR with more than 100 changed
files or more than 100 reviews silently truncated when the REST detail
fetch fell back to GraphQL -- with no way to even detect it. Add
pageInfo { hasNextPage endCursor } to both connections and walk each
independently with cursor pagination, bounded by a new
PR_DETAIL_GRAPHQL_MAX_PAGES constant mirroring the REST path's
PR_DETAIL_MAX_PAGES. A later-page failure keeps the items already
fetched instead of discarding them, matching githubPaginatedList's
partial-result semantics; a page-1 failure still surfaces the same way
to callers as before.
Closes JSONbored#7453
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7493 +/- ##
==========================================
- Coverage 91.41% 91.39% -0.02%
==========================================
Files 717 717
Lines 73032 73033 +1
Branches 21636 21636
==========================================
- Hits 66762 66752 -10
- Misses 5227 5234 +7
- Partials 1043 1047 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-20 13:12:14 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
fetchPullRequestDetailsFromGraphQl(the GraphQL fallback used byfetchPullRequestFiles/fetchPullRequestReviewswhen the REST-sidegithubPaginatedListfails to fetch page 1) requestedfiles(first: 100)andreviews(first: 100)with nopageInfo, so a PR with more than 100 changed files or more than 100 reviews was silently truncated with no way to even detect it — unlike the REST path, which already followsLink: rel="next"up toPR_DETAIL_MAX_PAGES = 10pages.pageInfo { hasNextPage endCursor }to both thefilesandreviewsGraphQL connections, and now walk each connection independently with cursor pagination (after: <cursor>), bounded by a newPR_DETAIL_GRAPHQL_MAX_PAGES = 10constant next to the function (same 10-page/1,000-item ceiling as the REST path'sPR_DETAIL_MAX_PAGES).githubPaginatedList's existing "keep a successful partial result" semantics. A page-1 failure still throws/propagates exactly as before, so the function's total-failure contract tofetchPullRequestFiles/fetchPullRequestReviews(which.catch(() => undefined)this call and fall back to a warning + empty result) is unchanged.GitHubPullRequestDetailsResponse's type to carrypageInfofor both connections.Closes #7453
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run typecheck(whole-repotsc --noEmit) reliably OOMs on this shared sandbox. Instead I ran a scopedtsc --noEmitagainstsrc/github/backfill.ts,src/env.d.ts,worker-configuration.d.ts, and the two changed/affected test files using the roottsconfig.json's exact compiler flags (strict,exactOptionalPropertyTypes,noUncheckedIndexedAccess,noImplicitOverride,noFallthroughCasesInSwitch,forceConsistentCasingInFileNames,skipLibCheck, target/lib/module/moduleResolution/types matching root config) — zero errors.test:workers,build:mcp,test:mcp-pack,ui:openapi:check,ui:lint,ui:typecheck,ui:buildare not applicable: this change is confined tosrc/github/backfill.ts(a Worker-side GraphQL query) and its unit tests — it touches no Cloudflare Worker entrypoints, MCP package, OpenAPI schema, orapps/loopover-ui/**code.fetchPullRequestDetailsFromGraphQlsit within the function's pre-existing/* v8 ignore start/stop */block (already excluded from Codecov before this PR, per its own comment: "GitHub detail GraphQL sparse-node fallbacks are exercised through PR detail hydration tests"). They are exercised by the two new regression tests below but are not counted toward the branch-coverage percentage, matching the file's existing convention for this fallback.Safety
UI Evidencesection below with screenshots. (N/A — no visible UI changes.)UI Evidence
N/A — backend-only change, no visible UI/frontend/docs changes.
Notes
test/unit/backfill.test.ts, mirroring the existingbackfillOpenPullRequestDetailsGraphQL-fallback test shape:paginates the GraphQL PR-detail fallback past 100 files instead of truncating at the first page (#7453)— a two-page fixture (100 files + 1 file on a second page reached via a mockedafter: "files-page-2"cursor) asserts all 101 files are persisted, not truncated at 100.does not send a second GraphQL PR-detail page when the fallback's files/reviews already fit on page one— asserts the existing single-page (hasNextPage: false) behavior is unaffected and no extra GraphQL request is made.