diff --git a/packages/plugins/scm-github/src/graphql-batch.ts b/packages/plugins/scm-github/src/graphql-batch.ts index cf92add29..b80e196c4 100644 --- a/packages/plugins/scm-github/src/graphql-batch.ts +++ b/packages/plugins/scm-github/src/graphql-batch.ts @@ -391,6 +391,15 @@ export function _resetBatchEnrichPRFailedEmittedForTesting(): void { */ export const MAX_BATCH_SIZE = 25; +/** + * Maximum number of status check contexts to include per PR in the GraphQL batch. + * Covers AO's current 18-check CI with small headroom while keeping batch cost bounded. + * At MAX_BATCH_SIZE (25 PRs), this yields at most 20 × 25 = 500 context nodes per query, + * well within GitHub's per-call node limits. + * Repos with more contexts still fall back safely via pageInfo.hasNextPage. + */ +export const CI_CONTEXTS_FIRST = 20; + /** * Check if an HTTP response contains a 304 Not Modified status. * Handles HTTP/1.1, HTTP/2, and HTTP/2.0 status lines. @@ -647,11 +656,10 @@ const PR_FIELDS = ` commit { statusCheckRollup { state - # 11 keeps per-PR node cost under budget for 25-PR batch queries - # (total cost ≤5000). Repos with >11 checks lose individual check - # visibility, but the rollup "state" still reflects all checks — - # overall pass/fail detection remains correct. - contexts(first: 11) { + # Fetch enough contexts for AO's normal 18-check CI while keeping + # the per-PR node cost bounded. If more contexts exist, pageInfo + # forces REST fallback for complete individual check details. + contexts(first: ${CI_CONTEXTS_FIRST}) { nodes { ... on CheckRun { name @@ -897,9 +905,9 @@ function parseCIState(statusCheckRollup: unknown): CIStatus { const rollup = statusCheckRollup as Record; const state = typeof rollup["state"] === "string" ? rollup["state"].toUpperCase() : ""; - // Map GitHub's statusCheckRollup.state to our CIStatus enum - // This top-level state aggregates all individual checks and is - // significantly cheaper than fetching contexts (10 points vs 50+ per PR) + // Map GitHub's statusCheckRollup.state to our CIStatus enum. + // This top-level state aggregates all individual checks; individual context + // details are parsed separately and only trusted when the page is complete. if (state === "SUCCESS") return "passing"; if (state === "FAILURE") return "failing"; if (state === "ERROR") return "failing"; @@ -984,10 +992,10 @@ function extractPREnrichment( const statusCheckRollup = commits?.nodes?.[0]?.commit?.statusCheckRollup; const ciStatus = statusCheckRollup ? parseCIState(statusCheckRollup) : "none"; - // Only include ciChecks when the list is complete (no truncation). - // contexts(first: 20) silently truncates PRs with >20 checks — when truncated, - // the failing check may be missing, so we set ciChecks to undefined to force - // the getCIChecks() REST fallback in maybeDispatchCIFailureDetails. + // Only include ciChecks when the bounded contexts page is complete. + // GitHub silently truncates larger context lists — when truncated, the failing + // check may be missing, so set ciChecks to undefined to force the getCIChecks() + // REST fallback in maybeDispatchCIFailureDetails. const contextsField = statusCheckRollup?.["contexts"] as Record | undefined; const pageInfo = contextsField?.["pageInfo"]; const contextsHasNextPage = diff --git a/packages/plugins/scm-github/test/graphql-batch.test.ts b/packages/plugins/scm-github/test/graphql-batch.test.ts index 2c987ea71..a714c93d9 100644 --- a/packages/plugins/scm-github/test/graphql-batch.test.ts +++ b/packages/plugins/scm-github/test/graphql-batch.test.ts @@ -1,10 +1,9 @@ /** * Unit tests for GraphQL batch PR enrichment. * - * Note: The GraphQL batch query was optimized to use only the top-level - * statusCheckRollup.state field instead of fetching individual contexts. - * This reduces GraphQL API cost from ~50 points to ~10 points per PR while - * providing the same semantic information for CI status determination. + * Note: The GraphQL batch query uses the top-level statusCheckRollup.state + * field for overall CI status and fetches a bounded contexts page for + * individual check details when the page is complete. */ import { describe, it, expect, beforeEach, vi } from "vitest"; @@ -13,6 +12,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { generateBatchQuery, MAX_BATCH_SIZE, + CI_CONTEXTS_FIRST, parseCIState, parseReviewDecision, parsePRState, @@ -164,6 +164,7 @@ describe("GraphQL Batch Query Generation", () => { expect(query).toContain("reviewDecision"); expect(query).toContain("commits"); expect(query).toContain("statusCheckRollup"); + expect(query).toContain(`contexts(first: ${CI_CONTEXTS_FIRST})`); }); it("should use sequential numeric aliases", () => { @@ -277,9 +278,9 @@ describe("CI State Parsing", () => { expect(parseCIState({ state: "EXPECTED" })).toBe("pending"); }); - it("should parse individual contexts for detailed state", () => { - // After optimization, we no longer fetch individual contexts. - // The top-level state provides the same semantic information. + it("should use aggregate state even when individual contexts are present", () => { + // The top-level state provides overall CI status. Individual context + // details are parsed separately for ciChecks when the page is complete. expect(parseCIState({ state: "PENDING", contexts: { @@ -719,10 +720,14 @@ describe("PR Enrichment Data Extraction", () => { }); }); -describe("MAX_BATCH_SIZE constant", () => { - it("should be defined as 25", () => { +describe("GraphQL batch sizing constants", () => { + it("should keep PR batches at 25 for optimal batch sizing", () => { expect(MAX_BATCH_SIZE).toBe(25); }); + + it("should request enough CI contexts for AO's 18-check CI", () => { + expect(CI_CONTEXTS_FIRST).toBe(20); + }); }); describe("ETag Cache", () => { @@ -1743,7 +1748,7 @@ describe("extractPREnrichment ciChecks", () => { contexts: { nodes: [ { name: "check-1", status: "COMPLETED", conclusion: "FAILURE", detailsUrl: null }, - // ... 19 more checks truncated + // ... more checks truncated beyond CI_CONTEXTS_FIRST ], pageInfo: { hasNextPage: true }, // list was truncated! },