Skip to content

fix(github): supplementOpenIssuesFromGraphQl is missing the endCursor null-guard its PR-side twin has #8312

Description

@JSONbored

Context

src/github/backfill.ts has two near-identical GraphQL pagination helpers that supplement REST-undercounted open issues/PRs: supplementOpenIssuesFromGraphQl (line 1780) and supplementOpenPullRequestsFromGraphQl (line 1874). Both loop through GraphQL pages using pageInfo.hasNextPage/pageInfo.endCursor, both are called from supplementUnderCountIfNeeded when a REST-based sync undercounts the GitHub-reported total, and both are wrapped in /* v8 ignore start */ ... /* v8 ignore stop */ blocks (documented as "covered by sparse-payload backfill tests" — i.e., deliberately excluded from the Codecov branch-count gate, matching this file's own established convention for this class of defensive GraphQL-payload-normalization code).

The two loops end their pagination check differently:

// src/github/backfill.ts:1831 (issues — MISSING the endCursor guard)
if (!issues?.pageInfo?.hasNextPage) break;
after = `, after: ${JSON.stringify(issues.pageInfo.endCursor)}`;
// src/github/backfill.ts:1897 (pull requests — HAS the guard)
if (!pullRequests?.pageInfo?.hasNextPage || !pullRequests.pageInfo.endCursor) break;
after = `, after: ${JSON.stringify(pullRequests.pageInfo.endCursor)}`;

Per git log, supplementOpenIssuesFromGraphQl was added first (commit a613357f6); supplementOpenPullRequestsFromGraphQl was added two days later (commit 38826a967) as a close copy, and picked up the || !pullRequests.pageInfo.endCursor guard the issues version never got — the fix was never backported.

This codebase already treats "hasNextPage: true with a missing/null endCursor" as a real, guarded-against GitHub API anomaly elsewhere — e.g. fetchLiveReviewThreadBlockers (line ~3976): if (!nextCursor || seenCursors.has(nextCursor)) break;, with a dedicated regression test ("stops review-thread pagination when GitHub omits the next cursor", test/unit/backfill-2.test.ts).

Without the guard, if GitHub ever returns hasNextPage: true with a null/absent endCursor on the issues path, JSON.stringify(undefined) serializes to the string undefined, producing a malformed next-page query (, after: undefined). That request fails, supplementOpenIssuesFromGraphQl throws, and supplementUnderCountIfNeeded's catch (line ~1770) discards the ENTIRE supplement attempt — including every issue already fetched on prior pages in this call — falling back to just a generic warning, instead of the graceful partial-result stop its sibling function achieves via the guard.

Requirements

  • supplementOpenIssuesFromGraphQl's pagination loop must stop (not continue with a malformed query) when GitHub reports hasNextPage: true but endCursor is missing/null — mirroring supplementOpenPullRequestsFromGraphQl's existing guard exactly (same condition shape: !issues?.pageInfo?.hasNextPage || !issues.pageInfo.endCursor).
  • On hitting this condition, the function must still return supplemented with whatever count was accumulated from prior pages (the existing return supplemented; at the end of the loop already does this once the loop breaks — no other logic change needed beyond the break condition itself).
  • Do not change the function's /* v8 ignore start/stop */ scope or remove it — this fix stays inside the existing ignored block, matching its sibling.

Deliverables

  • src/github/backfill.ts:1831: change if (!issues?.pageInfo?.hasNextPage) break; to if (!issues?.pageInfo?.hasNextPage || !issues.pageInfo.endCursor) break;, matching line 1897's condition exactly.
  • A regression test in test/unit/backfill.test.ts, alongside the existing "supplements REST undercounts from sparse GraphQL open-data payloads" test (~line 4182): mock the LoopOverOpenIssuesSupplement GraphQL response to return pageInfo: { hasNextPage: true, endCursor: null } on the first page, and assert supplementOpenIssuesFromGraphQl returns the partial count already accumulated (or the segment completes as partial with no thrown/malformed-query error) rather than surfacing a GraphQL-request-failure warning — mirroring the equivalent PR-side test's shape for supplementOpenPullRequestsFromGraphQl.

Test Coverage Requirements

This function sits inside an existing /* v8 ignore start/stop */ block (matching the sibling PR-supplement function's own treatment), so this specific line is intentionally outside Codecov's branch-count gate — note this explicitly in the PR so it isn't mistaken for uncovered patch code. The regression test above is still required as real behavioral coverage (this file's own convention already tests ignored-block code via dedicated unit tests, e.g. the review-thread cursor-guard test cited above), it just won't move the Codecov percentage.

Expected Outcome

supplementOpenIssuesFromGraphQl degrades gracefully (keeps whatever it already fetched, stops cleanly) on the same GitHub GraphQL anomaly its sibling supplementOpenPullRequestsFromGraphQl already handles, instead of throwing away an entire in-progress supplement pass on a malformed follow-up query.

Links & Resources

  • src/github/backfill.ts:1780-1833 (supplementOpenIssuesFromGraphQl, the function to fix)
  • src/github/backfill.ts:1874-1901 (supplementOpenPullRequestsFromGraphQl, the correct sibling to mirror)
  • src/github/backfill.ts:~3976 (fetchLiveReviewThreadBlockers's analogous cursor guard, same anomaly class)
  • test/unit/backfill.test.ts (existing issues-supplement pagination tests, ~line 4182)
  • test/unit/backfill-2.test.ts (the review-thread cursor-guard regression test, for the expected test shape)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions