feat(github): collapse per-PR hydration N+1 with a GraphQL batch query - #86
Merged
Conversation
Each poll hydrated every distinct open PR with up to 3 REST calls (detail + reviews + check-runs) plus a per-repo permission fetch — an N+1 that could exhaust the core 5000-req/hr limit on a large inbox. Add a batched `pullRequestBatch` to the `GitHubAPI` protocol backed by one GraphQL query per account: it fetches detail + reviews + check-runs + the viewer's repo merge signals for all PRs at once, then maps the result back onto the existing REST value types so the store's `deriveGate`/`ciRollup` logic consumes it unchanged (a transport swap, not a logic rewrite). On by default; any GraphQL failure (a GHE server missing a field, a transport error) auto-falls-back to the per-PR REST path per account, and per-node nulls degrade to a REST retry for that PR. - `GitHubGraphQLQuery.swift`: aliased batch query builder + chunking. - `GraphQLPRMapping.swift`: response decode + mapping (incl. legacy StatusContext → CheckRun, mergeStateStatus → mergeable_state). - `AppConfig.graphQLURL(forAPI:)`: public + GHE endpoint derivation. - Store folds each bundle's repo merge info into `repoMergeInfo` so a later gate-only refresh after approve still finds it. Closes #83
…ch REST
The GraphQL statusCheckRollup returns both CheckRun and legacy StatusContext
(commit-status API) nodes, but the REST commits/{sha}/check-runs endpoint the
fallback path uses returns only CheckRuns. Consuming StatusContext made the
batch path's CI state diverge from its REST fallback — a status-only repo would
flash a CI dot that vanished on any poll that fell back to REST. Filter to
CheckRun nodes so both paths agree. Extending CI to commit statuses would need
both paths changed together (follow-up).
- Strip redundant explicit Sendable + wrap single-line bodies flagged by CI's newer swiftformat (the local .claude worktree exclude had hidden them). - graphQLURL: an api.-prefixed host (public or GHE Cloud data-residency api.<tenant>.ghe.com) serves GraphQL at /graphql, not /api/graphql — key on the host prefix so data-residency tenants hit the batch path instead of 404→REST. - Add a FakeGitHubAPI omit knob + AppStoreTests case exercising the per-node partial fallback (one PR resolves via GraphQL, an omitted one falls back to per-PR REST in the same wave).
Merge of stage (#85) added a required gateIssueSeq to fold(); the GraphQL batch path now ticks nextGateWriteSeq() per PR before issuing the batch and passes it when folding, so a batch-hydrated gate obeys the same merge-poll write-preserve ordering as the REST drain (#84). Fixes the CI build/test break from the stale base.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Lands the structural fix for #83: one batched GraphQL query per account replaces the per-PR REST hydration N+1.
Each poll hydrated every distinct open PR with up to 3 REST calls (
GET pulls/{n}+commits/{sha}/check-runs+pulls/{n}/reviews) plus a per-repo permission fetch — on a large inbox (measured ~193 req for one cold refresh of a 76-PR inbox) this could exhaust the core 5000-req/hr limit. A newpullRequestBatchon theGitHubAPIprotocol fetches detail + reviews + check-runs + the viewer's repo merge signals for all PRs in one query.Design principle: transport swap, not a logic rewrite
The GraphQL client maps the response back onto the existing decoded value types (
PullRequestDetail/PullRequestReview/CheckRun/RepoMergeInfo), so the store's unchangedderiveGate/ciRollup/checkRowModelderivation consumes a batched result identically. Everything downstream (gates, CI banners, hydration marks, notifications) is untouched.Fallback (GHE + robustness)
mergeStateStatus, a transport error) auto-falls-back to the per-PR REST path per account.hydrateRepoPermissionsand theupdated_atshort-circuit) is retained verbatim as the fallback.Files
GitHubGraphQLQuery.swift(new) — aliased batch query builder + internal chunking (25 PRs/query).GraphQLPRMapping.swift(new) — response decode + mapping (incl. legacyStatusContext→CheckRun,mergeStateStatus→mergeable_state,viewerPermission→canMerge).AppConfig.graphQLURL(forAPI:)— public + GHE endpoint derivation.AppStoreHydration.swift— GraphQL fast path in the drain with REST fallback; folds each bundle's repo merge info intorepoMergeInfoso a later gate-only refresh after approve still finds it.AppStore.useGraphQLBatchflag (default true; tests flip it to exercise the REST short-circuit).Tests
GraphQLPRMappingTests— decode/map a canned response: bundle fields,mergeStateStatusmapping table, CheckRun + StatusContext → rollup, per-node null tolerance, top-levelerrors→ throw, query builder + chunking + slug split.AppConfigWebHostTests— GraphQL endpoint (public + GHE + port + fallback).AppStoreTests— batch path hydrates via one round-trip (no per-PR REST), andbatchError→ REST fallback still hydrates.useGraphQLBatch = false.just checkclean.Closes #83