Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions packages/plugins/scm-github/src/graphql-batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
whoisasx marked this conversation as resolved.

/**
* Check if an HTTP response contains a 304 Not Modified status.
* Handles HTTP/1.1, HTTP/2, and HTTP/2.0 status lines.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -897,9 +905,9 @@ function parseCIState(statusCheckRollup: unknown): CIStatus {
const rollup = statusCheckRollup as Record<string, unknown>;
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";
Expand Down Expand Up @@ -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<string, unknown> | undefined;
const pageInfo = contextsField?.["pageInfo"];
const contextsHasNextPage =
Expand Down
25 changes: 15 additions & 10 deletions packages/plugins/scm-github/test/graphql-batch.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -13,6 +12,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
import {
generateBatchQuery,
MAX_BATCH_SIZE,
CI_CONTEXTS_FIRST,
parseCIState,
parseReviewDecision,
parsePRState,
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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!
},
Expand Down
Loading