diff --git a/.gitignore b/.gitignore index 090f848..9006715 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ dist/ hack/ .serena/ .playwright-mcp/ +playwright-report/ +test-results/ diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index addd234..4d3e819 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -1,5 +1,5 @@ import { createSignal, createMemo, Show, Switch, Match, onMount, onCleanup } from "solid-js"; -import { createStore } from "solid-js/store"; +import { createStore, produce } from "solid-js/store"; import Header from "../layout/Header"; import TabBar, { TabId } from "../layout/TabBar"; import FilterBar from "../layout/FilterBar"; @@ -82,17 +82,81 @@ async function pollFetch(): Promise { setDashboardData("loading", true); } try { - const data = await fetchAllData(); + // Two-phase rendering: phase 1 callback fires with light issues + PRs + // so the UI renders immediately. Phase 2 (enrichment + workflow runs) + // arrives when fetchAllData resolves. + let phaseOneFired = false; + const data = await fetchAllData((lightData) => { + // Phase 1: render light issues + PRs immediately — but only on initial + // load (no cached data). On reload with cached data, the cache already + // has enriched PRs; replacing them with light PRs would cause a visible + // flicker (badges disappear then reappear when phase 2 arrives). + if (dashboardData.pullRequests.length === 0) { + phaseOneFired = true; + setDashboardData({ + issues: lightData.issues, + pullRequests: lightData.pullRequests, + loading: false, + lastRefreshedAt: new Date(), + }); + } + }); // When notifications gate says nothing changed, keep existing data if (!data.skipped) { const now = new Date(); - setDashboardData({ - issues: data.issues, - pullRequests: data.pullRequests, - workflowRuns: data.workflowRuns, - loading: false, - lastRefreshedAt: now, - }); + + if (phaseOneFired) { + // Phase 1 fired — use fine-grained merge for the light→enriched + // transition. Only update heavy fields to avoid re-rendering the + // entire list (light fields haven't changed within this poll cycle). + const enrichedMap = new Map(); + for (const pr of data.pullRequests) enrichedMap.set(pr.id, pr); + + setDashboardData(produce((state) => { + state.issues = data.issues; + state.workflowRuns = data.workflowRuns; + state.loading = false; + state.lastRefreshedAt = now; + + let canMerge = state.pullRequests.length === enrichedMap.size; + if (canMerge) { + for (let i = 0; i < state.pullRequests.length; i++) { + if (!enrichedMap.has(state.pullRequests[i].id)) { canMerge = false; break; } + } + } + + if (canMerge) { + for (let i = 0; i < state.pullRequests.length; i++) { + const e = enrichedMap.get(state.pullRequests[i].id)!; + const pr = state.pullRequests[i]; + pr.headSha = e.headSha; + pr.assigneeLogins = e.assigneeLogins; + pr.reviewerLogins = e.reviewerLogins; + pr.checkStatus = e.checkStatus; + pr.additions = e.additions; + pr.deletions = e.deletions; + pr.changedFiles = e.changedFiles; + pr.comments = e.comments; + pr.reviewThreads = e.reviewThreads; + pr.totalReviewCount = e.totalReviewCount; + pr.enriched = e.enriched; + } + } else { + state.pullRequests = data.pullRequests; + } + })); + } else { + // Phase 1 did NOT fire (cached data existed or subsequent poll). + // Full atomic replacement — all fields (light + heavy) may have + // changed since the last cycle. + setDashboardData({ + issues: data.issues, + pullRequests: data.pullRequests, + workflowRuns: data.workflowRuns, + loading: false, + lastRefreshedAt: now, + }); + } // Persist for stale-while-revalidate on full page reload. // Errors are transient and not persisted. Deferred to avoid blocking paint. const cachePayload = { diff --git a/src/app/components/dashboard/PullRequestsTab.tsx b/src/app/components/dashboard/PullRequestsTab.tsx index f06e5fb..22362ed 100644 --- a/src/app/components/dashboard/PullRequestsTab.tsx +++ b/src/app/components/dashboard/PullRequestsTab.tsx @@ -149,9 +149,13 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { const roles = deriveInvolvementRoles(props.userLogin, pr.userLogin, pr.assigneeLogins, pr.reviewerLogins); const sizeCategory = prSizeCategory(pr.additions, pr.deletions); - // Tab filters + // Tab filters — light-field filters always apply; heavy-field filters + // only apply to enriched PRs so unenriched phase-1 PRs aren't incorrectly hidden + const isEnriched = pr.enriched !== false; if (tabFilters.role !== "all") { - if (!roles.includes(tabFilters.role as "author" | "reviewer" | "assignee")) return false; + // Role depends on assigneeLogins/reviewerLogins (heavy), but "author" is light + if (isEnriched && !roles.includes(tabFilters.role as "author" | "reviewer" | "assignee")) return false; + if (!isEnriched && tabFilters.role === "author" && !roles.includes("author")) return false; } if (tabFilters.reviewDecision !== "all") { if (pr.reviewDecision !== tabFilters.reviewDecision) return false; @@ -160,14 +164,14 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { if (tabFilters.draft === "draft" && !pr.draft) return false; if (tabFilters.draft === "ready" && pr.draft) return false; } - if (tabFilters.checkStatus !== "all") { + if (tabFilters.checkStatus !== "all" && isEnriched) { if (tabFilters.checkStatus === "none") { if (pr.checkStatus !== null) return false; } else { if (pr.checkStatus !== tabFilters.checkStatus) return false; } } - if (tabFilters.sizeCategory !== "all") { + if (tabFilters.sizeCategory !== "all" && isEnriched) { if (sizeCategory !== tabFilters.sizeCategory) return false; } @@ -429,29 +433,33 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { createdAt={pr.createdAt} url={pr.htmlUrl} labels={pr.labels} - commentCount={pr.comments + pr.reviewThreads} + commentCount={pr.enriched !== false ? pr.comments + pr.reviewThreads : undefined} onIgnore={() => handleIgnore(pr)} density={config.viewDensity} >
- + + + - - - - - - Merge conflict - + + + + + + + Merge conflict + + Draft - 0}> + 0}> Reviewers: {pr.reviewerLogins.slice(0, 5).join(", ")} {pr.reviewerLogins.length > 5 && ` +${pr.reviewerLogins.length - 5} more`} diff --git a/src/app/services/api.ts b/src/app/services/api.ts index c9f84fa..692eb37 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -65,6 +65,8 @@ export interface PullRequest { labels: { name: string; color: string }[]; reviewDecision: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null; totalReviewCount: number; + /** False when only light fields are loaded (phase 1); true/undefined when fully enriched */ + enriched?: boolean; } export interface WorkflowRun { @@ -138,8 +140,10 @@ interface RawWorkflowRun { // ── Constants ──────────────────────────────────────────────────────────────── -// Batch repos into chunks for search queries (keeps URL length manageable) -const SEARCH_REPO_BATCH_SIZE = 30; +// Batch repos into chunks for search queries (keeps URL length manageable). +// 50 repos keeps query strings well within GitHub's limits (qualifiers like repo: +// are excluded from the 256-char search term cap) while reducing HTTP round-trips. +const SEARCH_REPO_BATCH_SIZE = 50; // Max fork PRs per GraphQL batch for the statusCheckRollup fallback query. // Each alias looks up a single commit in the fork repo. Kept conservatively small @@ -203,6 +207,37 @@ function chunkArray(arr: T[], size: number): T[][] { type GitHubOctokit = NonNullable>; +/** + * Runs an array of async task factories with bounded concurrency. + * Unlike chunked Promise.allSettled, tasks start immediately as slots free up + * rather than waiting for an entire chunk to finish. + */ +async function pooledAllSettled( + tasks: (() => Promise)[], + concurrency: number +): Promise[]> { + const results: PromiseSettledResult[] = new Array(tasks.length); + let nextIndex = 0; + + async function runWorker(): Promise { + while (nextIndex < tasks.length) { + const idx = nextIndex++; + try { + results[idx] = { status: "fulfilled", value: await tasks[idx]() }; + } catch (reason) { + results[idx] = { status: "rejected", reason }; + } + } + } + + const workers = Array.from( + { length: Math.min(concurrency, tasks.length) }, + () => runWorker() + ); + await Promise.all(workers); + return results; +} + // ── GraphQL search types ───────────────────────────────────────────────────── interface GraphQLIssueNode { @@ -296,7 +331,7 @@ interface ForkQueryResponse { const ISSUES_SEARCH_QUERY = ` query($q: String!, $cursor: String) { - search(query: $q, type: ISSUE, first: 100, after: $cursor) { + search(query: $q, type: ISSUE, first: 50, after: $cursor) { issueCount pageInfo { hasNextPage endCursor } nodes { @@ -309,8 +344,8 @@ const ISSUES_SEARCH_QUERY = ` createdAt updatedAt author { login avatarUrl } - labels(first: 20) { nodes { name color } } - assignees(first: 20) { nodes { login } } + labels(first: 10) { nodes { name color } } + assignees(first: 10) { nodes { login } } repository { nameWithOwner } comments { totalCount } } @@ -323,7 +358,7 @@ const ISSUES_SEARCH_QUERY = ` const PR_SEARCH_QUERY = ` query($q: String!, $cursor: String) { # GitHub search API uses type: ISSUE for both issues and PRs - search(query: $q, type: ISSUE, first: 100, after: $cursor) { + search(query: $q, type: ISSUE, first: 50, after: $cursor) { issueCount pageInfo { hasNextPage endCursor } nodes { @@ -343,19 +378,19 @@ const PR_SEARCH_QUERY = ` headRepository { owner { login } nameWithOwner } repository { nameWithOwner } mergeStateStatus - assignees(first: 20) { nodes { login } } - reviewRequests(first: 20) { + assignees(first: 10) { nodes { login } } + reviewRequests(first: 10) { # Team reviewers are excluded (only User fragment matched) nodes { requestedReviewer { ... on User { login } } } } - labels(first: 20) { nodes { name color } } + labels(first: 10) { nodes { name color } } additions deletions changedFiles comments { totalCount } reviewThreads { totalCount } reviewDecision - latestReviews(first: 15) { + latestReviews(first: 5) { totalCount nodes { author { login } } } @@ -373,6 +408,211 @@ const PR_SEARCH_QUERY = ` } `; +// ── GraphQL combined search query ───────────────────────────────────────────── + +// ── Two-phase rendering: light + heavy queries ─────────────────────────────── + +const LIGHT_PR_FRAGMENT = ` + fragment LightPRFields on PullRequest { + id + databaseId + number + title + state + isDraft + url + createdAt + updatedAt + author { login avatarUrl } + repository { nameWithOwner } + headRefName + baseRefName + reviewDecision + labels(first: 10) { nodes { name color } } + } +`; + +/** + * Phase 1 query: fetches issues fully and PRs with minimal fields. + * PR enrichment (check status, size, reviewers, etc.) comes from phase 2. + */ +const LIGHT_COMBINED_SEARCH_QUERY = ` + query($issueQ: String!, $prInvQ: String!, $prRevQ: String!, + $issueCursor: String, $prInvCursor: String, $prRevCursor: String) { + issues: search(query: $issueQ, type: ISSUE, first: 50, after: $issueCursor) { + issueCount + pageInfo { hasNextPage endCursor } + nodes { + ... on Issue { + databaseId + number + title + state + url + createdAt + updatedAt + author { login avatarUrl } + labels(first: 10) { nodes { name color } } + assignees(first: 10) { nodes { login } } + repository { nameWithOwner } + comments { totalCount } + } + } + } + prInvolves: search(query: $prInvQ, type: ISSUE, first: 50, after: $prInvCursor) { + issueCount + pageInfo { hasNextPage endCursor } + nodes { + ... on PullRequest { + ...LightPRFields + } + } + } + prReviewReq: search(query: $prRevQ, type: ISSUE, first: 50, after: $prRevCursor) { + issueCount + pageInfo { hasNextPage endCursor } + nodes { + ... on PullRequest { + ...LightPRFields + } + } + } + rateLimit { remaining resetAt } + } + ${LIGHT_PR_FRAGMENT} +`; + +/** Standalone light PR search query for pagination follow-ups. */ +const LIGHT_PR_SEARCH_QUERY = ` + query($q: String!, $cursor: String) { + search(query: $q, type: ISSUE, first: 50, after: $cursor) { + issueCount + pageInfo { hasNextPage endCursor } + nodes { + ... on PullRequest { + ...LightPRFields + } + } + } + rateLimit { remaining resetAt } + } + ${LIGHT_PR_FRAGMENT} +`; + +interface LightPRSearchResponse { + search: { + issueCount: number; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + nodes: (GraphQLLightPRNode | null)[]; + }; + rateLimit?: { remaining: number; resetAt: string }; +} + +/** Phase 2 backfill query: enriches PRs with heavy fields using node IDs. */ +const HEAVY_PR_BACKFILL_QUERY = ` + query($ids: [ID!]!) { + nodes(ids: $ids) { + ... on PullRequest { + databaseId + headRefOid + headRepository { owner { login } nameWithOwner } + mergeStateStatus + assignees(first: 10) { nodes { login } } + reviewRequests(first: 10) { + nodes { requestedReviewer { ... on User { login } } } + } + latestReviews(first: 5) { + totalCount + nodes { author { login } } + } + additions + deletions + changedFiles + comments { totalCount } + reviewThreads { totalCount } + commits(last: 1) { + nodes { + commit { + statusCheckRollup { state } + } + } + } + } + } + rateLimit { remaining resetAt } + } +`; + +interface GraphQLLightPRNode { + id: string; // GraphQL global node ID + databaseId: number; + number: number; + title: string; + state: string; + isDraft: boolean; + url: string; + createdAt: string; + updatedAt: string; + author: { login: string; avatarUrl: string } | null; + repository: { nameWithOwner: string } | null; + headRefName: string; + baseRefName: string; + reviewDecision: string | null; + labels: { nodes: { name: string; color: string }[] }; +} + +interface GraphQLHeavyPRNode { + databaseId: number; + headRefOid: string; + headRepository: { owner: { login: string }; nameWithOwner: string } | null; + mergeStateStatus: string; + assignees: { nodes: { login: string }[] }; + reviewRequests: { nodes: { requestedReviewer: { login: string } | null }[] }; + latestReviews: { + totalCount: number; + nodes: { author: { login: string } | null }[]; + }; + additions: number; + deletions: number; + changedFiles: number; + comments: { totalCount: number }; + reviewThreads: { totalCount: number }; + commits: { + nodes: { + commit: { + statusCheckRollup: { state: string } | null; + }; + }[]; + }; +} + +interface LightCombinedSearchResponse { + issues: { + issueCount: number; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + nodes: (GraphQLIssueNode | null)[]; + }; + prInvolves: { + issueCount: number; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + nodes: (GraphQLLightPRNode | null)[]; + }; + prReviewReq: { + issueCount: number; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + nodes: (GraphQLLightPRNode | null)[]; + }; + rateLimit?: { remaining: number; resetAt: string }; +} + +interface HeavyBackfillResponse { + nodes: (GraphQLHeavyPRNode | null)[]; + rateLimit?: { remaining: number; resetAt: string }; +} + +// Max node IDs per nodes() query (GitHub limit) +const NODES_BATCH_SIZE = 100; + // ── GraphQL search functions ────────────────────────────────────────────────── interface SearchPageResult { @@ -395,8 +635,9 @@ async function paginateGraphQLSearch boolean, // returns true if node was added (for cap counting) currentCount: () => number, cap: number, + startCursor?: string | null, ): Promise<{ capReached: boolean }> { - let cursor: string | null = null; + let cursor: string | null = startCursor ?? null; let capReached = false; while (true) { @@ -464,6 +705,499 @@ function buildRepoQualifiers(repos: RepoRef[]): string { .join(" "); } +/** + * Processes a single GraphQL issue node into the app's Issue shape. + * Returns true if the node was added (not a duplicate or invalid). + */ +function processIssueNode( + node: GraphQLIssueNode, + seen: Set, + issues: Issue[] +): boolean { + if (node.databaseId == null || !node.repository) return false; + if (seen.has(node.databaseId)) return false; + seen.add(node.databaseId); + issues.push({ + id: node.databaseId, + number: node.number, + title: node.title, + state: node.state, + htmlUrl: node.url, + createdAt: node.createdAt, + updatedAt: node.updatedAt, + userLogin: node.author?.login ?? "", + userAvatarUrl: node.author?.avatarUrl ?? "", + labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })), + assigneeLogins: node.assignees.nodes.map((a) => a.login), + repoFullName: node.repository.nameWithOwner, + comments: node.comments.totalCount, + }); + return true; +} + +/** + * Runs the fork PR statusCheckRollup fallback for PRs where head repo owner + * differs from base repo owner (fork PRs). Mutates prMap in place. + */ +async function runForkPRFallback( + octokit: GitHubOctokit, + prMap: Map, + forkInfoMap: Map +): Promise { + const forkCandidates: ForkCandidate[] = []; + for (const [databaseId, pr] of prMap) { + if (pr.checkStatus !== null) continue; + const headInfo = forkInfoMap.get(databaseId); + if (!headInfo) continue; + const baseOwner = pr.repoFullName.split("/")[0].toLowerCase(); + if (headInfo.owner.toLowerCase() === baseOwner) continue; + forkCandidates.push({ databaseId, headOwner: headInfo.owner, headRepo: headInfo.repoName, sha: pr.headSha }); + } + + if (forkCandidates.length === 0) return; + + const forkChunks = chunkArray(forkCandidates, GRAPHQL_CHECK_BATCH_SIZE); + await Promise.allSettled(forkChunks.map(async (forkChunk) => { + const varDefs: string[] = []; + const variables: Record = {}; + const fragments: string[] = []; + + for (let i = 0; i < forkChunk.length; i++) { + varDefs.push(`$owner${i}: String!`, `$repo${i}: String!`, `$sha${i}: String!`); + variables[`owner${i}`] = forkChunk[i].headOwner; + variables[`repo${i}`] = forkChunk[i].headRepo; + variables[`sha${i}`] = forkChunk[i].sha; + fragments.push( + `fork${i}: repository(owner: $owner${i}, name: $repo${i}) { + object(expression: $sha${i}) { + ... on Commit { + statusCheckRollup { state } + } + } + }` + ); + } + + const forkQuery = `query(${varDefs.join(", ")}) {\n${fragments.join("\n")}\nrateLimit { remaining resetAt }\n}`; + + try { + const forkResponse = await octokit.graphql(forkQuery, variables); + if (forkResponse.rateLimit) updateGraphqlRateLimit(forkResponse.rateLimit as { remaining: number; resetAt: string }); + + for (let i = 0; i < forkChunk.length; i++) { + const data = forkResponse[`fork${i}`] as ForkRepoResult | null | undefined; + const state = data?.object?.statusCheckRollup?.state ?? null; + const pr = prMap.get(forkChunk[i].databaseId); + if (pr) pr.checkStatus = mapCheckStatus(state); + } + } catch (err) { + const partialData = (err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object") + ? err.data as Record + : null; + + if (partialData) { + for (let i = 0; i < forkChunk.length; i++) { + const data = partialData[`fork${i}`]; + if (!data) continue; + const state = data.object?.statusCheckRollup?.state ?? null; + const pr = prMap.get(forkChunk[i].databaseId); + if (pr) pr.checkStatus = mapCheckStatus(state); + } + } + + console.warn("[api] Fork PR statusCheckRollup fallback partially failed:", err); + pushNotification("graphql", "Fork PR check status unavailable — CI status may be missing for some PRs", "warning"); + } + })); +} + +// ── Combined search (issues + PRs in single request) ───────────────────────── + +export interface FetchIssuesAndPRsResult { + issues: Issue[]; + pullRequests: PullRequest[]; + errors: ApiError[]; +} + +// ── Two-phase: light combined search ────────────────────────────────────────── + +interface LightSearchResult { + issues: Issue[]; + pullRequests: PullRequest[]; + /** GraphQL node IDs for phase 2 backfill */ + prNodeIds: string[]; + errors: ApiError[]; +} + +/** + * Processes a light PR node into a PullRequest with default values for heavy fields. + * Returns true if the node was added (not a duplicate or invalid). + * Stores the GraphQL node ID in nodeIdMap for later backfill. + */ +function processLightPRNode( + node: GraphQLLightPRNode, + prMap: Map, + nodeIdMap: Map +): boolean { + if (node.databaseId == null || !node.repository) return false; + if (prMap.has(node.databaseId)) return false; + + nodeIdMap.set(node.databaseId, node.id); + prMap.set(node.databaseId, { + id: node.databaseId, + number: node.number, + title: node.title, + state: node.state, + draft: node.isDraft, + htmlUrl: node.url, + createdAt: node.createdAt, + updatedAt: node.updatedAt, + userLogin: node.author?.login ?? "", + userAvatarUrl: node.author?.avatarUrl ?? "", + headSha: "", + headRef: node.headRefName, + baseRef: node.baseRefName, + assigneeLogins: [], + reviewerLogins: [], + repoFullName: node.repository.nameWithOwner, + checkStatus: null, + additions: 0, + deletions: 0, + changedFiles: 0, + comments: 0, + reviewThreads: 0, + labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })), + reviewDecision: mapReviewDecision(node.reviewDecision), + totalReviewCount: 0, + enriched: false, + }); + return true; +} + +/** + * Phase 1: light combined search. Fetches issues fully and PRs with minimal fields. + * Returns light PRs (enriched: false) and their GraphQL node IDs for phase 2 backfill. + */ +async function graphqlLightCombinedSearch( + octokit: GitHubOctokit, + repos: RepoRef[], + userLogin: string +): Promise { + if (!VALID_LOGIN.test(userLogin)) { + return { + issues: [], + pullRequests: [], + prNodeIds: [], + errors: [{ repo: "search", statusCode: null, message: "Invalid userLogin", retryable: false }], + }; + } + + const chunks = chunkArray(repos, SEARCH_REPO_BATCH_SIZE); + const issueSeen = new Set(); + const issues: Issue[] = []; + const prMap = new Map(); + const nodeIdMap = new Map(); + const errors: ApiError[] = []; + const ISSUE_CAP = 1000; + const PR_CAP = 1000; + + const chunkResults = await Promise.allSettled(chunks.map(async (chunk, chunkIdx) => { + const repoQualifiers = buildRepoQualifiers(chunk); + const issueQ = `is:issue is:open involves:${userLogin} ${repoQualifiers}`; + const prInvQ = `is:pr is:open involves:${userLogin} ${repoQualifiers}`; + const prRevQ = `is:pr is:open review-requested:${userLogin} ${repoQualifiers}`; + const batchLabel = `light-batch-${chunkIdx + 1}/${chunks.length}`; + + let response: LightCombinedSearchResponse; + let isPartial = false; + try { + try { + response = await octokit.graphql(LIGHT_COMBINED_SEARCH_QUERY, { + issueQ, prInvQ, prRevQ, + issueCursor: null, prInvCursor: null, prRevCursor: null, + }); + } catch (err) { + const partial = (err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object") + ? err.data as Partial + : null; + if (partial && (partial.issues || partial.prInvolves || partial.prReviewReq)) { + response = { + issues: partial.issues ?? { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prInvolves: partial.prInvolves ?? { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prReviewReq: partial.prReviewReq ?? { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: partial.rateLimit, + }; + isPartial = true; + const { message } = extractRejectionError(err); + errors.push({ repo: batchLabel, statusCode: null, message, retryable: true }); + } else { + throw err; + } + } + + if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit); + + for (const node of response.issues.nodes) { + if (issues.length >= ISSUE_CAP) break; + if (!node) continue; + processIssueNode(node, issueSeen, issues); + } + + for (const node of response.prInvolves.nodes) { + if (prMap.size >= PR_CAP) break; + if (!node) continue; + processLightPRNode(node, prMap, nodeIdMap); + } + for (const node of response.prReviewReq.nodes) { + if (prMap.size >= PR_CAP) break; + if (!node) continue; + processLightPRNode(node, prMap, nodeIdMap); + } + + if (isPartial) return; + + // Pagination follow-ups for issues (PRs don't paginate in light mode — + // backfill handles the full set via node IDs) + if (response.issues.pageInfo.hasNextPage && response.issues.pageInfo.endCursor && issues.length < ISSUE_CAP) { + await paginateGraphQLSearch( + octokit, ISSUES_SEARCH_QUERY, issueQ, batchLabel, errors, + (node) => processIssueNode(node, issueSeen, issues), + () => issues.length, ISSUE_CAP, response.issues.pageInfo.endCursor, + ); + } + + // Light PR pagination: re-fetch with light query for additional pages + const prPaginationTasks: Promise[] = []; + if (response.prInvolves.pageInfo.hasNextPage && response.prInvolves.pageInfo.endCursor && prMap.size < PR_CAP) { + prPaginationTasks.push(paginateGraphQLSearch( + octokit, LIGHT_PR_SEARCH_QUERY, prInvQ, batchLabel, errors, + (node) => processLightPRNode(node, prMap, nodeIdMap), + () => prMap.size, PR_CAP, response.prInvolves.pageInfo.endCursor, + )); + } + if (response.prReviewReq.pageInfo.hasNextPage && response.prReviewReq.pageInfo.endCursor && prMap.size < PR_CAP) { + prPaginationTasks.push(paginateGraphQLSearch( + octokit, LIGHT_PR_SEARCH_QUERY, prRevQ, batchLabel, errors, + (node) => processLightPRNode(node, prMap, nodeIdMap), + () => prMap.size, PR_CAP, response.prReviewReq.pageInfo.endCursor, + )); + } + if (prPaginationTasks.length > 0) { + await Promise.allSettled(prPaginationTasks); + } + } catch (err) { + const { statusCode, message } = extractRejectionError(err); + errors.push({ repo: batchLabel, statusCode, message, retryable: statusCode === null || statusCode >= 500 }); + } + })); + + for (const result of chunkResults) { + if (result.status === "rejected") { + const { statusCode, message } = extractRejectionError(result.reason); + errors.push({ repo: "light-batch", statusCode, message, retryable: statusCode === null || statusCode >= 500 }); + } + } + + if (issues.length >= ISSUE_CAP) { + console.warn(`[api] Issue search results capped at ${ISSUE_CAP}`); + pushNotification("search/issues", `Issue search results capped at 1,000 — some items are hidden`, "warning"); + issues.splice(ISSUE_CAP); + } + + if (prMap.size >= PR_CAP) { + console.warn(`[api] PR search results capped at ${PR_CAP}`); + pushNotification("search/prs", `PR search results capped at 1,000 — some items are hidden`, "warning"); + } + + const pullRequests = [...prMap.values()]; + if (pullRequests.length >= PR_CAP) pullRequests.splice(PR_CAP); + const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null); + + return { issues, pullRequests, prNodeIds, errors }; +} + +// ── Two-phase: heavy backfill ───────────────────────────────────────────────── + +export interface PREnrichmentData { + id: number; + headSha: string; + headRepository: { owner: string; repoName: string } | null; + assigneeLogins: string[]; + reviewerLogins: string[]; + checkStatus: CheckStatus["status"]; + additions: number; + deletions: number; + changedFiles: number; + comments: number; + reviewThreads: number; + totalReviewCount: number; +} + +/** + * Phase 2: fetches heavy PR fields using `nodes(ids: [...])` query. + * Returns enrichment data keyed by databaseId. + */ +export async function fetchPREnrichment( + octokit: GitHubOctokit, + nodeIds: string[] +): Promise<{ enrichments: Map; errors: ApiError[] }> { + const enrichments = new Map(); + const errors: ApiError[] = []; + + if (nodeIds.length === 0) return { enrichments, errors }; + + const batches = chunkArray(nodeIds, NODES_BATCH_SIZE); + await Promise.allSettled(batches.map(async (batch, batchIdx) => { + try { + const response = await octokit.graphql(HEAVY_PR_BACKFILL_QUERY, { + ids: batch, + }); + + if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit); + + for (const node of response.nodes) { + if (!node || node.databaseId == null) continue; + + const pendingLogins = node.reviewRequests.nodes + .map((n) => n.requestedReviewer?.login) + .filter((l): l is string => l != null); + const actualLogins = node.latestReviews.nodes + .map((n) => n.author?.login) + .filter((l): l is string => l != null); + const reviewerLogins = [...new Set([...pendingLogins, ...actualLogins].map(l => l.toLowerCase()))]; + + let checkStatus = mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null); + const mss = node.mergeStateStatus; + if (mss === "DIRTY" || mss === "BEHIND") { + checkStatus = "conflict"; + } else if (mss === "UNSTABLE") { + checkStatus = "failure"; + } + + let headRepository: PREnrichmentData["headRepository"] = null; + if (node.headRepository) { + const parts = node.headRepository.nameWithOwner.split("/"); + if (parts.length === 2) { + headRepository = { owner: node.headRepository.owner.login, repoName: parts[1] }; + } + } + + enrichments.set(node.databaseId, { + id: node.databaseId, + headSha: node.headRefOid, + headRepository, + assigneeLogins: node.assignees.nodes.map((a) => a.login), + reviewerLogins, + checkStatus, + additions: node.additions, + deletions: node.deletions, + changedFiles: node.changedFiles, + comments: node.comments.totalCount, + reviewThreads: node.reviewThreads.totalCount, + totalReviewCount: node.latestReviews.totalCount, + }); + } + } catch (err) { + const { statusCode, message } = extractRejectionError(err); + errors.push({ + repo: `backfill-batch-${batchIdx + 1}/${batches.length}`, + statusCode, message, + retryable: statusCode === null || statusCode >= 500, + }); + } + })); + + return { enrichments, errors }; +} + +/** + * Merges phase 2 enrichment data into light PRs. Returns enriched PR array. + * Also detects fork PRs for the statusCheckRollup fallback. + */ +function mergeEnrichment( + lightPRs: PullRequest[], + enrichments: Map, + forkInfoMap: Map +): PullRequest[] { + return lightPRs.map((pr) => { + const e = enrichments.get(pr.id); + if (!e) return pr; // Keep enriched: false if backfill missed this PR + + if (e.headRepository) { + forkInfoMap.set(pr.id, e.headRepository); + } + + return { + ...pr, + headSha: e.headSha, + assigneeLogins: e.assigneeLogins, + reviewerLogins: e.reviewerLogins, + checkStatus: e.checkStatus, + additions: e.additions, + deletions: e.deletions, + changedFiles: e.changedFiles, + comments: e.comments, + reviewThreads: e.reviewThreads, + totalReviewCount: e.totalReviewCount, + enriched: true, + }; + }); +} + +/** + * Fetches open issues and PRs using a two-phase approach: + * - Phase 1 (light): minimal fields for immediate rendering + * - Phase 2 (heavy): enrichment via nodes(ids:[]) backfill + * + * If onLightData is provided, it fires after phase 1 with light data + * so the UI can render immediately. The returned promise resolves + * with fully enriched data after phase 2 completes. + */ +export async function fetchIssuesAndPullRequests( + octokit: ReturnType, + repos: RepoRef[], + userLogin: string, + onLightData?: (data: FetchIssuesAndPRsResult) => void, +): Promise { + if (!octokit) throw new Error("No GitHub client available"); + if (repos.length === 0 || !userLogin) return { issues: [], pullRequests: [], errors: [] }; + + // Phase 1: light combined search + const lightResult = await graphqlLightCombinedSearch(octokit, repos, userLogin); + + // Notify caller with light data for immediate rendering + if (onLightData) { + onLightData({ + issues: lightResult.issues, + pullRequests: lightResult.pullRequests, + errors: lightResult.errors, + }); + } + + // Phase 2: heavy backfill + if (lightResult.prNodeIds.length === 0) { + return { + issues: lightResult.issues, + pullRequests: lightResult.pullRequests.map(pr => ({ ...pr, enriched: true })), + errors: lightResult.errors, + }; + } + + const { enrichments, errors: backfillErrors } = await fetchPREnrichment(octokit, lightResult.prNodeIds); + const forkInfoMap = new Map(); + const enrichedPRs = mergeEnrichment(lightResult.pullRequests, enrichments, forkInfoMap); + + // Fork PR fallback for enriched PRs + const prMap = new Map(enrichedPRs.map(pr => [pr.id, pr])); + await runForkPRFallback(octokit, prMap, forkInfoMap); + + return { + issues: lightResult.issues, + pullRequests: [...prMap.values()], + errors: [...lightResult.errors, ...backfillErrors], + }; +} + /** * Fetches open issues via GraphQL search, using cursor-based pagination. * Batches repos into chunks of SEARCH_REPO_BATCH_SIZE and runs chunks in parallel. @@ -887,10 +1621,8 @@ export async function fetchWorkflowRuns( const targetRunsPerRepo = maxWorkflows * maxRuns; const perPage = Math.min(Math.max(targetRunsPerRepo + 5, 20), 100); - const RUNS_CONCURRENCY = 10; - const repoChunks = chunkArray(repos, RUNS_CONCURRENCY); - for (const chunk of repoChunks) { - const chunkResults = await Promise.allSettled(chunk.map(async (repo) => { + const RUNS_CONCURRENCY = 20; + const tasks = repos.map((repo) => async () => { // Skip repos known to have zero workflow runs (cached empty result) if (_emptyActionRepos.has(repo.fullName)) return; @@ -971,13 +1703,13 @@ export async function fetchWorkflowRuns( }); } } - })); + }); - for (const result of chunkResults) { - if (result.status === "rejected") { - const { statusCode, message } = extractRejectionError(result.reason); - allErrors.push({ repo: "workflow-runs", statusCode, message, retryable: true }); - } + const taskResults = await pooledAllSettled(tasks, RUNS_CONCURRENCY); + for (const result of taskResults) { + if (result.status === "rejected") { + const { statusCode, message } = extractRejectionError(result.reason); + allErrors.push({ repo: "workflow-runs", statusCode, message, retryable: true }); } } diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index c61332e..bcc126e 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -3,8 +3,7 @@ import { getClient } from "./github"; import { config } from "../stores/config"; import { user, onAuthCleared } from "../stores/auth"; import { - fetchIssues, - fetchPullRequests, + fetchIssuesAndPullRequests, fetchWorkflowRuns, type Issue, type PullRequest, @@ -116,7 +115,15 @@ const MAX_GATE_STALENESS_MS = 10 * 60 * 1000; // 10 minutes // ── fetchAllData orchestrator ───────────────────────────────────────────────── -export async function fetchAllData(): Promise { +/** + * Fetches all dashboard data. Supports two-phase progressive rendering: + * - If onLightData is provided, fires with light issues+PRs as soon as + * phase 1 completes (before enrichment and workflow runs finish). + * - The returned promise resolves with fully enriched data. + */ +export async function fetchAllData( + onLightData?: (data: DashboardData) => void, +): Promise { const octokit = getClient(); if (!octokit) { return { issues: [], pullRequests: [], workflowRuns: [], errors: [], skipped: true }; @@ -138,23 +145,26 @@ export async function fetchAllData(): Promise { const repos = config.selectedRepos; const userLogin = user()?.login ?? ""; - // Note: NOT using updated:>= or created:>= filters on any endpoint because - // the dashboard uses full-replacement — each poll replaces all data. Date filters - // would cause unchanged items to vanish from the display. ETag caching already - // handles the "nothing changed" case for workflow runs (304 = free). - - // Issues + PRs use GraphQL (5000 pts/hr), workflow runs use REST core (5000/hr) — all parallel - const [issueResult, prResult, runResult] = await Promise.allSettled([ - fetchIssues(octokit, repos, userLogin), - fetchPullRequests(octokit, repos, userLogin), + // Issues + PRs use a two-phase approach: light query first (phase 1), + // then heavy backfill (phase 2). Workflow runs use REST core. + // All streams run in parallel (GraphQL 5000 pts/hr + REST core 5000/hr). + const [issuesAndPrsResult, runResult] = await Promise.allSettled([ + fetchIssuesAndPullRequests(octokit, repos, userLogin, onLightData ? (lightData) => { + // Phase 1: fire callback with light issues + PRs (no workflow runs yet) + onLightData({ + issues: lightData.issues, + pullRequests: lightData.pullRequests, + workflowRuns: [], + errors: lightData.errors, + }); + } : undefined), fetchWorkflowRuns(octokit, repos, config.maxWorkflowsPerRepo, config.maxRunsPerWorkflow), ]); // Collect top-level errors (total function failures) const topLevelErrors: ApiError[] = []; const settled: [PromiseSettledResult, string][] = [ - [issueResult, "issues"], - [prResult, "pull-requests"], + [issuesAndPrsResult, "issues-and-prs"], [runResult, "workflow-runs"], ]; for (const [result, label] of settled) { @@ -169,29 +179,27 @@ export async function fetchAllData(): Promise { } // Extract data and per-batch errors from successful results - const issueData = issueResult.status === "fulfilled" ? issueResult.value : null; - const prData = prResult.status === "fulfilled" ? prResult.value : null; + const issuesAndPrsData = issuesAndPrsResult.status === "fulfilled" ? issuesAndPrsResult.value : null; const runData = runResult.status === "fulfilled" ? runResult.value : null; // Merge all error sources: top-level failures + per-batch partial failures const errors = [ ...topLevelErrors, - ...(issueData?.errors ?? []), - ...(prData?.errors ?? []), + ...(issuesAndPrsData?.errors ?? []), ...(runData?.errors ?? []), ]; // Only activate the notifications gate if at least one fetch succeeded. - // If all three failed (e.g., network outage), we don't want the gate to + // If all failed (e.g., network outage), we don't want the gate to // suppress retries on the next poll cycle. - const anySucceeded = issueData !== null || prData !== null || runData !== null; + const anySucceeded = issuesAndPrsData !== null || runData !== null; if (anySucceeded) { _lastSuccessfulFetch = new Date(); } return { - issues: issueData?.issues ?? [], - pullRequests: prData?.pullRequests ?? [], + issues: issuesAndPrsData?.issues ?? [], + pullRequests: issuesAndPrsData?.pullRequests ?? [], workflowRuns: runData?.workflowRuns ?? [], errors, }; diff --git a/tests/services/api-optimization.test.ts b/tests/services/api-optimization.test.ts new file mode 100644 index 0000000..31c3afb --- /dev/null +++ b/tests/services/api-optimization.test.ts @@ -0,0 +1,677 @@ +import "fake-indexeddb/auto"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { + fetchIssues, + fetchPullRequests, + fetchIssuesAndPullRequests, + fetchWorkflowRuns, + type RepoRef, +} from "../../src/app/services/api"; +import { clearCache } from "../../src/app/stores/cache"; + +vi.mock("../../src/app/lib/errors", () => ({ + pushNotification: vi.fn(), + pushError: vi.fn(), + getErrors: vi.fn().mockReturnValue([]), + dismissError: vi.fn(), + getNotifications: vi.fn().mockReturnValue([]), + getUnreadCount: vi.fn().mockReturnValue(0), + markAllAsRead: vi.fn(), +})); + +// ── Fixtures ────────────────────────────────────────────────────────────────── + +const graphqlIssueNode = { + databaseId: 1347, + number: 1347, + title: "Found a bug", + state: "open", + url: "https://github.com/octocat/Hello-World/issues/1347", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://github.com/images/error/octocat_happy.gif" }, + labels: { nodes: [{ name: "bug", color: "d73a4a" }] }, + assignees: { nodes: [{ login: "octocat" }] }, + repository: { nameWithOwner: "octocat/Hello-World" }, + comments: { totalCount: 3 }, +}; + +/** Full PR node used by standalone fetchPullRequests (has all heavy fields) */ +const graphqlPRNode = { + databaseId: 42, + number: 42, + title: "Add feature", + state: "open", + isDraft: false, + url: "https://github.com/octocat/Hello-World/pull/42", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://github.com/images/error/octocat_happy.gif" }, + headRefOid: "abc123", + headRefName: "feature-branch", + baseRefName: "main", + headRepository: { owner: { login: "octocat" }, nameWithOwner: "octocat/Hello-World" }, + repository: { nameWithOwner: "octocat/Hello-World" }, + mergeStateStatus: "CLEAN", + assignees: { nodes: [{ login: "octocat" }] }, + reviewRequests: { nodes: [{ requestedReviewer: { login: "reviewer2" } }] }, + labels: { nodes: [{ name: "feature", color: "a2eeef" }] }, + additions: 100, + deletions: 20, + changedFiles: 5, + comments: { totalCount: 3 }, + reviewThreads: { totalCount: 2 }, + reviewDecision: "APPROVED", + latestReviews: { totalCount: 1, nodes: [{ author: { login: "reviewer1" } }] }, + commits: { nodes: [{ commit: { statusCheckRollup: { state: "SUCCESS" } } }] }, +}; + +/** Light PR node used by the two-phase combined query (phase 1) */ +function makeLightPRNode(overrides: Partial = {}) { + return { ...graphqlLightPRNodeDefaults, ...overrides }; +} + +const graphqlLightPRNodeDefaults = { + id: "PR_kwDOtest42", + databaseId: 42, + number: 42, + title: "Add feature", + state: "open", + isDraft: false, + url: "https://github.com/octocat/Hello-World/pull/42", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://github.com/images/error/octocat_happy.gif" }, + repository: { nameWithOwner: "octocat/Hello-World" }, + headRefName: "feature-branch", + baseRefName: "main", + reviewDecision: "APPROVED", + labels: { nodes: [{ name: "feature", color: "a2eeef" }] }, +}; + +/** Heavy PR node returned by phase 2 backfill (nodes(ids:[])) */ +function makeHeavyPRNode(databaseId: number, _nodeId?: string) { + return { + databaseId, + headRefOid: "abc123", + headRepository: { owner: { login: "octocat" }, nameWithOwner: "octocat/Hello-World" }, + mergeStateStatus: "CLEAN", + assignees: { nodes: [{ login: "octocat" }] }, + reviewRequests: { nodes: [{ requestedReviewer: { login: "reviewer2" } }] }, + latestReviews: { totalCount: 1, nodes: [{ author: { login: "reviewer1" } }] }, + additions: 100, + deletions: 20, + changedFiles: 5, + comments: { totalCount: 3 }, + reviewThreads: { totalCount: 2 }, + commits: { nodes: [{ commit: { statusCheckRollup: { state: "SUCCESS" } } }] }, + }; +} + +const rateLimit = { remaining: 4999, resetAt: new Date(Date.now() + 3600000).toISOString() }; + +function makeSearchResponse(nodes: T[], hasNextPage = false) { + return { + issueCount: nodes.length, + pageInfo: { hasNextPage, endCursor: hasNextPage ? "cursor1" : null }, + nodes, + }; +} + +function makeLightCombinedResponse( + issueNodes = [graphqlIssueNode], + prNodes = [makeLightPRNode()], + issueHasNext = false, + prInvHasNext = false, + prRevHasNext = false, +) { + return { + issues: makeSearchResponse(issueNodes, issueHasNext), + prInvolves: makeSearchResponse(prNodes, prInvHasNext), + prReviewReq: makeSearchResponse([], prRevHasNext), + rateLimit, + }; +} + +function makeHeavyBackfillResponse(prNodes: ReturnType[]) { + return { + nodes: prNodes, + rateLimit, + }; +} + +function makeRepos(count: number): RepoRef[] { + return Array.from({ length: count }, (_, i) => ({ + owner: "org", + name: `repo-${i}`, + fullName: `org/repo-${i}`, + })); +} + +type OctokitLike = ReturnType; + +/** + * Creates a mock octokit that handles both light combined and heavy backfill queries. + * Detects backfill calls by the presence of the `ids` variable. + */ +function makeTwoPhaseOctokit( + lightImpl: (query: string, variables?: unknown) => Promise, + heavyNodes?: ReturnType[], +) { + return { + request: vi.fn(async () => ({ data: [], headers: {} })), + graphql: vi.fn(async (query: string, variables?: Record) => { + // Heavy backfill query has `ids` variable + if (variables && "ids" in variables) { + return makeHeavyBackfillResponse(heavyNodes ?? []); + } + return lightImpl(query, variables); + }), + paginate: { iterator: vi.fn() }, + }; +} + +function makeOctokit(graphqlImpl: (query: string, variables?: unknown) => Promise) { + return { + request: vi.fn(async () => ({ data: [], headers: {} })), + graphql: vi.fn(graphqlImpl), + paginate: { iterator: vi.fn() }, + }; +} + +beforeEach(async () => { + await clearCache(); + vi.resetAllMocks(); +}); + +// ── Call count verification: combined vs separate ───────────────────────────── + +describe("API call count: combined vs separate", () => { + + describe("with 1-50 repos (single chunk)", () => { + const repos = makeRepos(30); + + it("separate fetchIssues + fetchPullRequests makes 3 GraphQL calls", async () => { + const octokit = makeOctokit(async () => ({ + search: makeSearchResponse([graphqlIssueNode]), + rateLimit, + })); + + await fetchIssues(octokit as unknown as OctokitLike, repos, "testuser"); + const issuesCalls = octokit.graphql.mock.calls.length; + + octokit.graphql.mockClear(); + await fetchPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + const prCalls = octokit.graphql.mock.calls.length; + + // Issues: 1 call, PRs: 2 calls (involves + review-requested) = 3 total + expect(issuesCalls).toBe(1); + expect(prCalls).toBe(2); + expect(issuesCalls + prCalls).toBe(3); + }); + + it("combined fetchIssuesAndPullRequests makes 2 GraphQL calls (light + heavy)", async () => { + const lightPR = makeLightPRNode(); + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse([graphqlIssueNode], [lightPR]), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // 1 light combined + 1 heavy backfill = 2 total + expect(octokit.graphql).toHaveBeenCalledTimes(2); + }); + + it("combined returns same data shape as separate calls", async () => { + // Separate calls + const separateOctokit = makeOctokit(async (_query, variables) => { + const q = (variables as Record).q as string; + if (q.includes("is:issue")) { + return { search: makeSearchResponse([graphqlIssueNode]), rateLimit }; + } + return { search: makeSearchResponse([graphqlPRNode]), rateLimit }; + }); + + const issueResult = await fetchIssues(separateOctokit as unknown as OctokitLike, repos, "testuser"); + const prResult = await fetchPullRequests(separateOctokit as unknown as OctokitLike, repos, "testuser"); + + // Combined call (two-phase) + const lightPR = makeLightPRNode(); + const combinedOctokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse([graphqlIssueNode], [lightPR]), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + const combinedResult = await fetchIssuesAndPullRequests(combinedOctokit as unknown as OctokitLike, repos, "testuser"); + + // Same data shape and content + expect(combinedResult.issues.length).toBe(issueResult.issues.length); + expect(combinedResult.pullRequests.length).toBe(prResult.pullRequests.length); + expect(combinedResult.issues[0].id).toBe(issueResult.issues[0].id); + expect(combinedResult.pullRequests[0].id).toBe(prResult.pullRequests[0].id); + // Enriched PRs should have enriched flag + expect(combinedResult.pullRequests[0].enriched).toBe(true); + }); + }); + + describe("with 51-100 repos (two chunks)", () => { + const repos = makeRepos(80); + + it("separate fetchIssues + fetchPullRequests makes 6 GraphQL calls", async () => { + const octokit = makeOctokit(async () => ({ + search: makeSearchResponse([{ ...graphqlIssueNode, databaseId: Math.random() * 100000 | 0 }]), + rateLimit, + })); + + await fetchIssues(octokit as unknown as OctokitLike, repos, "testuser"); + const issuesCalls = octokit.graphql.mock.calls.length; + + octokit.graphql.mockClear(); + await fetchPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + const prCalls = octokit.graphql.mock.calls.length; + + // Issues: 2 chunks × 1 call = 2, PRs: 2 chunks × 2 query types = 4 + expect(issuesCalls).toBe(2); + expect(prCalls).toBe(4); + expect(issuesCalls + prCalls).toBe(6); + }); + + it("combined fetchIssuesAndPullRequests makes 3 GraphQL calls (2 light + 1 heavy)", async () => { + let callId = 0; + const octokit = makeTwoPhaseOctokit( + async () => { + const id = ++callId; + return makeLightCombinedResponse( + [{ ...graphqlIssueNode, databaseId: id + 10000 }], + [makeLightPRNode({ databaseId: id, id: `PR_kwDO_${id}` })], + ); + }, + [makeHeavyPRNode(1, "PR_kwDO_1"), makeHeavyPRNode(2, "PR_kwDO_2")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // 2 light combined (1 per chunk) + 1 heavy backfill = 3 total + expect(octokit.graphql).toHaveBeenCalledTimes(3); + }); + }); + + describe("with 101-150 repos (three chunks)", () => { + const repos = makeRepos(120); + + it("separate makes 9 calls, combined makes 4", async () => { + let callId = 0; + const separateOctokit = makeOctokit(async () => ({ + search: makeSearchResponse([{ ...graphqlIssueNode, databaseId: ++callId }]), + rateLimit, + })); + + await fetchIssues(separateOctokit as unknown as OctokitLike, repos, "testuser"); + const issuesCalls = separateOctokit.graphql.mock.calls.length; + separateOctokit.graphql.mockClear(); + await fetchPullRequests(separateOctokit as unknown as OctokitLike, repos, "testuser"); + const prCalls = separateOctokit.graphql.mock.calls.length; + + // Issues: 3 chunks, PRs: 3 chunks × 2 = 6 + expect(issuesCalls).toBe(3); + expect(prCalls).toBe(6); + + callId = 0; + const combinedOctokit = makeTwoPhaseOctokit( + async () => { + const id = ++callId; + return makeLightCombinedResponse( + [{ ...graphqlIssueNode, databaseId: id + 20000 }], + [makeLightPRNode({ databaseId: id + 30000, id: `PR_kwDO_${id}` })], + ); + }, + [ + makeHeavyPRNode(30001, "PR_kwDO_1"), + makeHeavyPRNode(30002, "PR_kwDO_2"), + makeHeavyPRNode(30003, "PR_kwDO_3"), + ], + ); + + await fetchIssuesAndPullRequests(combinedOctokit as unknown as OctokitLike, repos, "testuser"); + + // 3 light combined (1 per chunk) + 1 heavy backfill = 4 total + expect(combinedOctokit.graphql).toHaveBeenCalledTimes(4); + }); + }); +}); + +// ── Pagination fallback verification ────────────────────────────────────────── + +describe("combined query pagination fallback", () => { + const repos = makeRepos(30); + + it("fires follow-up queries only for aliases that need pagination", async () => { + let callCount = 0; + const octokit = makeOctokit(async (_query, variables) => { + callCount++; + const vars = variables as Record; + + // Heavy backfill + if (vars && "ids" in vars) { + return makeHeavyBackfillResponse([makeHeavyPRNode(42, "PR_kwDOtest42")]); + } + + if (callCount === 1) { + // First call: light combined query. Issues need pagination, PRs don't. + return { + issues: makeSearchResponse([graphqlIssueNode], true), // hasNextPage + prInvolves: makeSearchResponse([makeLightPRNode()], false), + prReviewReq: makeSearchResponse([], false), + rateLimit, + }; + } + + // Follow-up: should be an individual issue search query (has cursor) + expect(vars.cursor).toBe("cursor1"); + expect(vars.q).toContain("is:issue"); + return { + search: makeSearchResponse([{ ...graphqlIssueNode, databaseId: 9999 }], false), + rateLimit, + }; + }); + + const result = await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // 1 light combined + 1 issue pagination + 1 heavy backfill = 3 total + expect(callCount).toBe(3); + expect(result.issues.length).toBe(2); // page 1 + page 2 + expect(result.pullRequests.length).toBe(1); + }); + + it("does not fire follow-up when no alias needs pagination", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse(), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // 1 light combined + 1 heavy backfill = 2 + expect(octokit.graphql).toHaveBeenCalledTimes(2); + }); +}); + +// ── Combined query sends correct query strings ──────────────────────────────── + +describe("combined query structure", () => { + const repos = makeRepos(5); + + it("sends all three search strings in one call with correct qualifiers", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse(), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // First call is the light combined query + const [, variables] = octokit.graphql.mock.calls[0] as [string, Record]; + + // Issue query string + expect(variables.issueQ).toContain("is:issue"); + expect(variables.issueQ).toContain("is:open"); + expect(variables.issueQ).toContain("involves:testuser"); + expect(variables.issueQ).toContain("repo:org/repo-0"); + + // PR involves query string + expect(variables.prInvQ).toContain("is:pr"); + expect(variables.prInvQ).toContain("involves:testuser"); + + // PR review-requested query string + expect(variables.prRevQ).toContain("is:pr"); + expect(variables.prRevQ).toContain("review-requested:testuser"); + }); + + it("uses GraphQL aliases (issues, prInvolves, prReviewReq) in the query", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse(), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + const [query] = octokit.graphql.mock.calls[0] as [string]; + expect(query).toContain("issues: search("); + expect(query).toContain("prInvolves: search("); + expect(query).toContain("prReviewReq: search("); + expect(query).toContain("LightPRFields"); + }); + + it("phase 2 sends nodes(ids:[]) backfill query", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse(), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // Second call is the heavy backfill + expect(octokit.graphql).toHaveBeenCalledTimes(2); + const [backfillQuery, backfillVars] = octokit.graphql.mock.calls[1] as [string, Record]; + expect(backfillQuery).toContain("nodes(ids:"); + expect(backfillVars.ids).toEqual(["PR_kwDOtest42"]); + }); +}); + +// ── Progressive rendering: onLightData callback ────────────────────────────── + +describe("onLightData callback (progressive rendering)", () => { + const repos = makeRepos(5); + + it("fires onLightData with light PRs (enriched: false) before phase 2 completes", async () => { + const callOrder: string[] = []; + const octokit = makeOctokit(async (_query, variables) => { + const vars = variables as Record; + if (vars && "ids" in vars) { + callOrder.push("heavy-start"); + return makeHeavyBackfillResponse([makeHeavyPRNode(42, "PR_kwDOtest42")]); + } + callOrder.push("light-start"); + return makeLightCombinedResponse(); + }); + + let lightDataReceived: Awaited> | null = null; + const result = await fetchIssuesAndPullRequests( + octokit as unknown as OctokitLike, + repos, + "testuser", + (data) => { + callOrder.push("onLightData"); + lightDataReceived = data; + }, + ); + + // onLightData fires after light query but before heavy backfill + expect(callOrder).toEqual(["light-start", "onLightData", "heavy-start"]); + + // Light data has PRs with enriched: false + expect(lightDataReceived).not.toBeNull(); + expect(lightDataReceived!.pullRequests.length).toBe(1); + expect(lightDataReceived!.pullRequests[0].enriched).toBe(false); + expect(lightDataReceived!.pullRequests[0].additions).toBe(0); // default heavy field + + // Final result has enriched PRs + expect(result.pullRequests[0].enriched).toBe(true); + expect(result.pullRequests[0].additions).toBe(100); // from heavy backfill + }); + + it("does not fire onLightData when callback is not provided", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse(), + [makeHeavyPRNode(42, "PR_kwDOtest42")], + ); + + // No callback — should not throw + const result = await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + expect(result.pullRequests[0].enriched).toBe(true); + }); + + it("marks PRs as enriched: true when there are 0 PRs (no backfill needed)", async () => { + const octokit = makeTwoPhaseOctokit( + async () => makeLightCombinedResponse([graphqlIssueNode], []), + [], + ); + + const result = await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + expect(result.pullRequests.length).toBe(0); + // Only light query, no backfill + expect(octokit.graphql).toHaveBeenCalledTimes(1); + }); +}); + +// ── Phase 2 backfill failure ─────────────────────────────────────────────────── + +describe("phase 2 backfill failure", () => { + const repos = makeRepos(5); + + it("returns light PRs with enriched: false when backfill fails entirely", async () => { + const octokit = makeOctokit(async (_query, variables) => { + const vars = variables as Record; + if (vars && "ids" in vars) { + throw new Error("GraphQL backfill network failure"); + } + return makeLightCombinedResponse(); + }); + + const result = await fetchIssuesAndPullRequests(octokit as unknown as OctokitLike, repos, "testuser"); + + // Issues should be complete + expect(result.issues.length).toBe(1); + // PRs returned but not enriched + expect(result.pullRequests.length).toBe(1); + expect(result.pullRequests[0].enriched).toBe(false); + expect(result.pullRequests[0].additions).toBe(0); // default heavy field + expect(result.pullRequests[0].checkStatus).toBeNull(); + // Backfill error should be in errors array + expect(result.errors.some(e => e.message.includes("backfill network failure"))).toBe(true); + }); +}); + +// ── Workflow run concurrency verification ───────────────────────────────────── + +describe("workflow run concurrency", () => { + it("starts all repo fetches concurrently up to concurrency limit", async () => { + const repos = makeRepos(25); + const concurrentPeak = { current: 0, max: 0 }; + + const octokit = { + request: vi.fn(async () => { + concurrentPeak.current++; + concurrentPeak.max = Math.max(concurrentPeak.max, concurrentPeak.current); + // Simulate network delay + await new Promise((r) => setTimeout(r, 10)); + concurrentPeak.current--; + return { + data: { workflow_runs: [], total_count: 0 }, + headers: { etag: "etag" }, + }; + }), + paginate: { iterator: vi.fn() }, + }; + + await fetchWorkflowRuns( + octokit as unknown as OctokitLike, + repos, + 5, + 3 + ); + + // Should reach concurrency > 10 (old limit was 10, new is 20) + // With 25 repos, all 25 should start within the 20-worker pool + expect(concurrentPeak.max).toBeGreaterThan(10); + expect(concurrentPeak.max).toBeLessThanOrEqual(20); + // All repos should be fetched + expect(octokit.request).toHaveBeenCalledTimes(25); + }); + + it("processes repos faster with pooled concurrency than sequential chunks", async () => { + const repos = makeRepos(30); + const DELAY_MS = 5; + + const makeTimedOctokit = () => ({ + request: vi.fn(async () => { + await new Promise((r) => setTimeout(r, DELAY_MS)); + return { + data: { workflow_runs: [], total_count: 0 }, + headers: { etag: "etag" }, + }; + }), + paginate: { iterator: vi.fn() }, + }); + + // Measure pooled approach (current implementation) + const pooledOctokit = makeTimedOctokit(); + const pooledStart = performance.now(); + await fetchWorkflowRuns(pooledOctokit as unknown as OctokitLike, repos, 5, 3); + const pooledDuration = performance.now() - pooledStart; + + // Sequential simulation: 3 batches of 10, each batch waits for all to finish + const sequentialStart = performance.now(); + for (let i = 0; i < 3; i++) { + await Promise.all( + repos.slice(i * 10, (i + 1) * 10).map(() => new Promise((r) => setTimeout(r, DELAY_MS))) + ); + } + const sequentialDuration = performance.now() - sequentialStart; + + // Pooled should be faster because it starts all 30 within 20-worker pool + // instead of waiting for 3 sequential batches of 10 + expect(pooledDuration).toBeLessThan(sequentialDuration); + }); +}); + +// ── Scaling: call count grows linearly with chunks ──────────────────────────── + +describe("scaling behavior", () => { + // Two-phase: each chunk needs 1 light query + 1 heavy backfill total + const repoCountsAndExpected = [ + { repos: 10, separateCalls: 3, combinedCalls: 2 }, // 1 light + 1 heavy + { repos: 50, separateCalls: 3, combinedCalls: 2 }, // 1 light + 1 heavy + { repos: 51, separateCalls: 6, combinedCalls: 3 }, // 2 light + 1 heavy + { repos: 100, separateCalls: 6, combinedCalls: 3 }, // 2 light + 1 heavy + { repos: 150, separateCalls: 9, combinedCalls: 4 }, // 3 light + 1 heavy + ]; + + for (const { repos: repoCount, separateCalls, combinedCalls } of repoCountsAndExpected) { + it(`${repoCount} repos: separate=${separateCalls} calls, combined=${combinedCalls} calls (${Math.round((1 - combinedCalls / separateCalls) * 100)}% reduction)`, async () => { + const repos = makeRepos(repoCount); + let nodeId = 0; + + // Count separate calls + const sepOctokit = makeOctokit(async () => ({ + search: makeSearchResponse([{ ...graphqlIssueNode, databaseId: ++nodeId }]), + rateLimit, + })); + await fetchIssues(sepOctokit as unknown as OctokitLike, repos, "testuser"); + const issueCalls = sepOctokit.graphql.mock.calls.length; + sepOctokit.graphql.mockClear(); + nodeId = 0; + await fetchPullRequests(sepOctokit as unknown as OctokitLike, repos, "testuser"); + const prCalls = sepOctokit.graphql.mock.calls.length; + expect(issueCalls + prCalls).toBe(separateCalls); + + // Count combined calls (two-phase) + nodeId = 0; + const combOctokit = makeTwoPhaseOctokit( + async () => { + const id = ++nodeId; + return makeLightCombinedResponse( + [{ ...graphqlIssueNode, databaseId: id + 40000 }], + [makeLightPRNode({ databaseId: id + 50000, id: `PR_kwDO_${id}` })], + ); + }, + // Generate heavy nodes for all expected chunks + Array.from({ length: Math.ceil(repoCount / 50) }, (_, i) => + makeHeavyPRNode(i + 1 + 50000, `PR_kwDO_${i + 1}`) + ), + ); + await fetchIssuesAndPullRequests(combOctokit as unknown as OctokitLike, repos, "testuser"); + expect(combOctokit.graphql).toHaveBeenCalledTimes(combinedCalls); + }); + } +}); diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index aed3840..1213d7b 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -279,8 +279,8 @@ describe("fetchIssues", () => { expect(octokit.graphql).not.toHaveBeenCalled(); }); - it("batches repos into chunks of 30", async () => { - const repos: RepoRef[] = Array.from({ length: 35 }, (_, i) => ({ + it("batches repos into chunks of 50", async () => { + const repos: RepoRef[] = Array.from({ length: 55 }, (_, i) => ({ owner: "org", name: `repo-${i}`, fullName: `org/repo-${i}`, @@ -293,7 +293,7 @@ describe("fetchIssues", () => { "octocat" ); - // Should make 2 GraphQL calls (30 + 5 repos) + // Should make 2 GraphQL calls (50 + 5 repos) expect(octokit.graphql).toHaveBeenCalledTimes(2); }); @@ -486,8 +486,8 @@ describe("fetchIssues", () => { it("truncates to exactly 1000 when parallel chunks overshoot", async () => { vi.mocked(pushNotification).mockClear(); - // 35 repos → 2 chunks. Each chunk returns 600 items (total 1200, well over cap). - const repos: RepoRef[] = Array.from({ length: 35 }, (_, i) => ({ + // 55 repos → 2 chunks. Each chunk returns 600 items (total 1200, well over cap). + const repos: RepoRef[] = Array.from({ length: 55 }, (_, i) => ({ owner: "org", name: `repo-${i}`, fullName: `org/repo-${i}`, diff --git a/tests/services/poll-fetchAllData.test.ts b/tests/services/poll-fetchAllData.test.ts index 0d926b0..4f4e9e4 100644 --- a/tests/services/poll-fetchAllData.test.ts +++ b/tests/services/poll-fetchAllData.test.ts @@ -23,10 +23,9 @@ vi.mock("../../src/app/stores/auth", () => ({ onAuthCleared: vi.fn(), })); -// Mock the three fetch functions +// Mock the fetch functions (combined issues+PRs and workflow runs) vi.mock("../../src/app/services/api", () => ({ - fetchIssues: vi.fn(), - fetchPullRequests: vi.fn(), + fetchIssuesAndPullRequests: vi.fn(), fetchWorkflowRuns: vi.fn(), resetEmptyActionRepos: vi.fn(), })); @@ -57,8 +56,7 @@ vi.mock("../../src/app/lib/errors", () => ({ // ── Helpers ─────────────────────────────────────────────────────────────────── -const emptyIssueResult = { issues: [], errors: [] }; -const emptyPrResult = { pullRequests: [], errors: [] }; +const emptyIssuesAndPrsResult = { issues: [], pullRequests: [], errors: [] }; const emptyRunResult = { workflowRuns: [], errors: [] }; function makeMockOctokit() { @@ -78,16 +76,14 @@ afterEach(() => { describe("fetchAllData — first call", () => { - it("returns data from all three fetches on first call", async () => { + it("returns data from all fetches on first call", async () => { vi.resetModules(); - // Re-import mocked modules after reset const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -102,15 +98,14 @@ describe("fetchAllData — first call", () => { expect(result.skipped).toBeUndefined(); }); - it("calls all three fetch functions on first call (no notification gate)", async () => { + it("calls both fetch functions on first call (no notification gate)", async () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -120,9 +115,8 @@ describe("fetchAllData — first call", () => { // First call: no _lastSuccessfulFetch, so notifications gate is skipped expect(mockOctokit.request).not.toHaveBeenCalled(); - // All three data fetches should run - expect(fetchIssues).toHaveBeenCalledOnce(); - expect(fetchPullRequests).toHaveBeenCalledOnce(); + // Both data fetches should run + expect(fetchIssuesAndPullRequests).toHaveBeenCalledOnce(); expect(fetchWorkflowRuns).toHaveBeenCalledOnce(); }); @@ -130,12 +124,11 @@ describe("fetchAllData — first call", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const { config } = await import("../../src/app/stores/config"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -143,8 +136,7 @@ describe("fetchAllData — first call", () => { await fetchAllData(); - expect(fetchIssues).toHaveBeenCalledWith(mockOctokit, config.selectedRepos, "octocat"); - expect(fetchPullRequests).toHaveBeenCalledWith(mockOctokit, config.selectedRepos, "octocat"); + expect(fetchIssuesAndPullRequests).toHaveBeenCalledWith(mockOctokit, config.selectedRepos, "octocat", undefined); expect(fetchWorkflowRuns).toHaveBeenCalledWith( mockOctokit, config.selectedRepos, @@ -157,11 +149,10 @@ describe("fetchAllData — first call", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -199,11 +190,10 @@ describe("fetchAllData — notification gate skip", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -212,8 +202,7 @@ describe("fetchAllData — notification gate skip", () => { // First call to set _lastSuccessfulFetch await fetchAllData(); - vi.mocked(fetchIssues).mockClear(); - vi.mocked(fetchPullRequests).mockClear(); + vi.mocked(fetchIssuesAndPullRequests).mockClear(); vi.mocked(fetchWorkflowRuns).mockClear(); // Simulate 304 from notifications — nothing changed @@ -223,8 +212,7 @@ describe("fetchAllData — notification gate skip", () => { expect(result.skipped).toBe(true); // Data fetches should NOT have been called - expect(fetchIssues).not.toHaveBeenCalled(); - expect(fetchPullRequests).not.toHaveBeenCalled(); + expect(fetchIssuesAndPullRequests).not.toHaveBeenCalled(); expect(fetchWorkflowRuns).not.toHaveBeenCalled(); }); @@ -233,11 +221,10 @@ describe("fetchAllData — notification gate skip", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -245,7 +232,7 @@ describe("fetchAllData — notification gate skip", () => { // First call — sets _lastSuccessfulFetch await fetchAllData(); - vi.mocked(fetchIssues).mockClear(); + vi.mocked(fetchIssuesAndPullRequests).mockClear(); // Advance time past 10 minutes vi.advanceTimersByTime(11 * 60 * 1000); @@ -257,28 +244,26 @@ describe("fetchAllData — notification gate skip", () => { // Should NOT be skipped — staleness cap bypasses the gate expect(result.skipped).toBeUndefined(); - expect(fetchIssues).toHaveBeenCalled(); + expect(fetchIssuesAndPullRequests).toHaveBeenCalled(); }); }); -// ── qa-1: All three fetches fail — errors aggregated, _lastSuccessfulFetch not updated ── +// ── qa-1: All fetches fail — errors aggregated, _lastSuccessfulFetch not updated ── describe("fetchAllData — all fetches fail", () => { - it("aggregates top-level errors when all three fetches reject", async () => { + it("aggregates top-level errors when all fetches reject", async () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockRejectedValue(Object.assign(new Error("Issues failed"), { status: 500 })); - vi.mocked(fetchPullRequests).mockRejectedValue(Object.assign(new Error("PRs failed"), { status: 500 })); + vi.mocked(fetchIssuesAndPullRequests).mockRejectedValue(Object.assign(new Error("Issues+PRs failed"), { status: 500 })); vi.mocked(fetchWorkflowRuns).mockRejectedValue(Object.assign(new Error("Runs failed"), { status: 500 })); const topLevelErrors = [ - { repo: "issues", statusCode: 500, message: "Issues failed", retryable: true }, - { repo: "pull-requests", statusCode: 500, message: "PRs failed", retryable: true }, + { repo: "issues-and-prs", statusCode: 500, message: "Issues+PRs failed", retryable: true }, { repo: "workflow-runs", statusCode: 500, message: "Runs failed", retryable: true }, ]; const { fetchAllData } = await import("../../src/app/services/poll"); @@ -292,16 +277,15 @@ describe("fetchAllData — all fetches fail", () => { expect(result.skipped).toBeUndefined(); }); - it("does NOT update _lastSuccessfulFetch when all three fetches reject", async () => { + it("does NOT update _lastSuccessfulFetch when all fetches reject", async () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockRejectedValue(new Error("fail")); - vi.mocked(fetchPullRequests).mockRejectedValue(new Error("fail")); + vi.mocked(fetchIssuesAndPullRequests).mockRejectedValue(new Error("fail")); vi.mocked(fetchWorkflowRuns).mockRejectedValue(new Error("fail")); const { fetchAllData } = await import("../../src/app/services/poll"); @@ -312,8 +296,7 @@ describe("fetchAllData — all fetches fail", () => { // Second call — if _lastSuccessfulFetch were set, a notification request would be made // Since all failed, it should NOT be set → no notification request mockOctokit.request.mockClear(); - vi.mocked(fetchIssues).mockRejectedValue(new Error("fail")); - vi.mocked(fetchPullRequests).mockRejectedValue(new Error("fail")); + vi.mocked(fetchIssuesAndPullRequests).mockRejectedValue(new Error("fail")); vi.mocked(fetchWorkflowRuns).mockRejectedValue(new Error("fail")); @@ -331,7 +314,7 @@ describe("fetchAllData — partial success", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); @@ -342,9 +325,8 @@ describe("fetchAllData — partial success", () => { userLogin: "octocat", userAvatarUrl: "", labels: [], assigneeLogins: [], repoFullName: "o/r", comments: 0, }]; - vi.mocked(fetchIssues).mockResolvedValue({ issues, errors: [] }); - vi.mocked(fetchPullRequests).mockRejectedValue(Object.assign(new Error("PR fetch failed"), { status: 503 })); - vi.mocked(fetchWorkflowRuns).mockResolvedValue({ workflowRuns: [], errors: [] }); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue({ issues, pullRequests: [], errors: [] }); + vi.mocked(fetchWorkflowRuns).mockRejectedValue(Object.assign(new Error("Runs failed"), { status: 503 })); const { fetchAllData } = await import("../../src/app/services/poll"); const result = await fetchAllData(); @@ -353,7 +335,7 @@ describe("fetchAllData — partial success", () => { expect(result.pullRequests).toEqual([]); expect(result.workflowRuns).toEqual([]); expect(result.errors).toHaveLength(1); - expect(result.errors[0].repo).toBe("pull-requests"); + expect(result.errors[0].repo).toBe("workflow-runs"); }); }); @@ -364,7 +346,7 @@ describe("fetchAllData — no client", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); vi.mocked(getClient).mockReturnValue(null); const { fetchAllData } = await import("../../src/app/services/poll"); @@ -375,7 +357,7 @@ describe("fetchAllData — no client", () => { expect(result.pullRequests).toEqual([]); expect(result.workflowRuns).toEqual([]); expect(result.errors).toEqual([]); - expect(fetchIssues).not.toHaveBeenCalled(); + expect(fetchIssuesAndPullRequests).not.toHaveBeenCalled(); }); }); @@ -386,12 +368,11 @@ describe("fetchAllData — resetPollState via onAuthCleared", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const { onAuthCleared } = await import("../../src/app/stores/auth"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -412,8 +393,7 @@ describe("fetchAllData — resetPollState via onAuthCleared", () => { // Gate is now disabled; third call should NOT call GET /notifications mockOctokit.request.mockClear(); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); expect(mockOctokit.request).not.toHaveBeenCalled(); @@ -423,8 +403,7 @@ describe("fetchAllData — resetPollState via onAuthCleared", () => { // First call after logout: _lastSuccessfulFetch is null → no gate check mockOctokit.request.mockClear(); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); // No notification gate on first call after reset (no _lastSuccessfulFetch) @@ -435,8 +414,7 @@ describe("fetchAllData — resetPollState via onAuthCleared", () => { data: [], headers: { "last-modified": "Thu, 20 Mar 2026 12:00:00 GMT" }, }); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); // GET /notifications was called — gate is active again (not disabled) @@ -454,11 +432,10 @@ describe("fetchAllData — If-Modified-Since header", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -473,8 +450,7 @@ describe("fetchAllData — If-Modified-Since header", () => { data: [], headers: { "last-modified": lastModified }, }); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); @@ -483,8 +459,7 @@ describe("fetchAllData — If-Modified-Since header", () => { data: [], headers: {}, }); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); @@ -505,12 +480,11 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const { pushNotification } = await import("../../src/app/lib/errors"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -518,7 +492,7 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { // First call — sets _lastSuccessfulFetch await fetchAllData(); - vi.mocked(fetchIssues).mockClear(); + vi.mocked(fetchIssuesAndPullRequests).mockClear(); // Second call — gate checks notifications, gets 403 mockOctokit.request.mockRejectedValueOnce({ status: 403 }); @@ -533,19 +507,16 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { // Third call — gate should be DISABLED, no notifications request mockOctokit.request.mockClear(); - vi.mocked(fetchIssues).mockClear(); - vi.mocked(fetchPullRequests).mockClear(); + vi.mocked(fetchIssuesAndPullRequests).mockClear(); vi.mocked(fetchWorkflowRuns).mockClear(); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); await fetchAllData(); expect(mockOctokit.request).not.toHaveBeenCalled(); - // The three data fetches still run - expect(fetchIssues).toHaveBeenCalled(); - expect(fetchPullRequests).toHaveBeenCalled(); + // The data fetches still run + expect(fetchIssuesAndPullRequests).toHaveBeenCalled(); expect(fetchWorkflowRuns).toHaveBeenCalled(); }); @@ -553,11 +524,10 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); - vi.mocked(fetchIssues).mockResolvedValue(emptyIssueResult); - vi.mocked(fetchPullRequests).mockResolvedValue(emptyPrResult); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue(emptyIssuesAndPrsResult); vi.mocked(fetchWorkflowRuns).mockResolvedValue(emptyRunResult); @@ -565,8 +535,7 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { // First call — sets _lastSuccessfulFetch await fetchAllData(); - vi.mocked(fetchIssues).mockClear(); - vi.mocked(fetchPullRequests).mockClear(); + vi.mocked(fetchIssuesAndPullRequests).mockClear(); vi.mocked(fetchWorkflowRuns).mockClear(); // Second call — gate returns 403; hasNotificationChanges returns true → full fetch runs @@ -575,8 +544,7 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { const result = await fetchAllData(); expect(result.skipped).toBeUndefined(); - expect(fetchIssues).toHaveBeenCalled(); - expect(fetchPullRequests).toHaveBeenCalled(); + expect(fetchIssuesAndPullRequests).toHaveBeenCalled(); expect(fetchWorkflowRuns).toHaveBeenCalled(); }); }); @@ -584,11 +552,11 @@ describe("fetchAllData — notification gate 403 auto-disable", () => { // ── qa-4: Concurrency verification ──────────────────────────────────────────── describe("fetchAllData — parallel execution", () => { - it("initiates all three fetches before any resolves", async () => { + it("initiates both fetches before either resolves", async () => { vi.resetModules(); const { getClient } = await import("../../src/app/services/github"); - const { fetchIssues, fetchPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); + const { fetchIssuesAndPullRequests, fetchWorkflowRuns } = await import("../../src/app/services/api"); const mockOctokit = makeMockOctokit(); vi.mocked(getClient).mockReturnValue(mockOctokit as unknown as ReturnType); @@ -596,13 +564,9 @@ describe("fetchAllData — parallel execution", () => { const resolvers: Array<(v: unknown) => void> = []; // Each mock records when it's called but doesn't resolve until manually triggered - vi.mocked(fetchIssues).mockImplementation(() => { - callOrder.push("issues-start"); - return new Promise((resolve) => { resolvers.push(() => resolve(emptyIssueResult)); }); - }); - vi.mocked(fetchPullRequests).mockImplementation(() => { - callOrder.push("prs-start"); - return new Promise((resolve) => { resolvers.push(() => resolve(emptyPrResult)); }); + vi.mocked(fetchIssuesAndPullRequests).mockImplementation(() => { + callOrder.push("issues-and-prs-start"); + return new Promise((resolve) => { resolvers.push(() => resolve(emptyIssuesAndPrsResult)); }); }); vi.mocked(fetchWorkflowRuns).mockImplementation(() => { callOrder.push("runs-start"); @@ -613,14 +577,14 @@ describe("fetchAllData — parallel execution", () => { const promise = fetchAllData(); - // Yield to allow Promise.allSettled to initiate all three + // Yield to allow Promise.allSettled to initiate both await new Promise((r) => setTimeout(r, 0)); - // All three should have been called BEFORE any resolved - expect(callOrder).toEqual(["issues-start", "prs-start", "runs-start"]); - expect(resolvers.length).toBe(3); + // Both should have been called BEFORE either resolved + expect(callOrder).toEqual(["issues-and-prs-start", "runs-start"]); + expect(resolvers.length).toBe(2); - // Now resolve all + // Now resolve both for (const resolve of resolvers) resolve(undefined); await promise; });