auto-claude: 155-fix-pr-list-diff-display-metrics (#1458)

* auto-claude: subtask-1-1 - Add GraphQL fetch helper function and update listPRs

- Add githubGraphQL helper function for making GraphQL API requests
- Add GraphQLPRListResponse interface for type safety
- Add LIST_PRS_QUERY GraphQL query to fetch PRs with additions/deletions/changedFiles
- Update listPRs handler to use GraphQL API instead of REST
- Import normalizeRepoReference from utils to parse owner/repo

The REST API /repos/{owner}/{repo}/pulls does NOT return diff stats
(additions/deletions/changedFiles). Only individual PR endpoints include
these fields. Switching to GraphQL solves this in a single request.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): add null check for repository and remove unused page param

- Add null check for response.data.repository before accessing pullRequests
  to prevent crash when repo doesn't exist or user lacks access
- Update GraphQLPRListResponse type to make repository nullable
- Remove misleading page parameter from listPRs handler since it was
  never used (always fetched first 100 PRs regardless of page value)
- Update preload API and hook to match new signature

Fixes PR review findings:
- Missing null check causes crash on non-existent repos
- Page parameter accepted but ignored breaks pagination

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): remove dead pagination code from PR list

- Remove unused page parameter from listPRs interface signature (NEW-001)
- Remove non-functional loadMore functionality since API fetches all PRs at once (NEW-002)
- Remove isLoadingMore, currentPage state and related infinite scroll code
- Simplify PRList component by removing unused pagination props
- Update UI to show "Showing first 100 PRs" when GitHub GraphQL limit is hit
- Clean up unused imports (useRef, useEffect, useCallback, Loader2)

The API fetches up to 100 open PRs in a single call (GitHub GraphQL limit).
The loadMore function was triggering redundant network requests returning
identical data. This cleanup removes the dead code from the incomplete
pagination refactor.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): cleanup dead code and use API hasNextPage

- QUAL-001: Remove unused viewportElement state and onViewportRef prop
  from PRList after pagination removal
- QUAL-002: Simplify GraphQL error message by removing verbose response body
- QUAL-003: Use actual hasNextPage from GitHub API instead of length heuristic
  - Add PRListResult interface with { prs, hasNextPage }
  - Update IPC handler to return pageInfo.hasNextPage from API
  - Update hook to use result.hasNextPage instead of result.length === 100

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): improve error handling and repo validation

- Use generic error messages in exceptions while logging details for debugging
- Add stricter validation for owner/repo format (must be exactly 2 parts)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-01-25 11:31:25 +01:00
committed by GitHub
parent d081af0422
commit 31f116db52
8 changed files with 204 additions and 156 deletions
@@ -19,7 +19,7 @@ import {
DEFAULT_FEATURE_THINKING,
} from "../../../shared/constants";
import type { AuthFailureInfo } from "../../../shared/types/terminal";
import { getGitHubConfig, githubFetch } from "./utils";
import { getGitHubConfig, githubFetch, normalizeRepoReference } from "./utils";
import { readSettingsFile } from "../../settings-utils";
import { getAugmentedEnv } from "../../env-utils";
import { getMemoryService, getDefaultDbPath } from "../../memory-service";
@@ -36,6 +36,105 @@ import {
buildRunnerArgs,
} from "./utils/subprocess-runner";
/**
* GraphQL response type for PR list query
* Note: repository can be null if the repo doesn't exist or user lacks access
*/
interface GraphQLPRListResponse {
data: {
repository: {
pullRequests: {
pageInfo: {
hasNextPage: boolean;
endCursor: string | null;
};
nodes: Array<{
number: number;
title: string;
body: string | null;
state: string;
author: { login: string } | null;
headRefName: string;
baseRefName: string;
additions: number;
deletions: number;
changedFiles: number;
assignees: { nodes: Array<{ login: string }> };
createdAt: string;
updatedAt: string;
url: string;
}>;
};
} | null;
};
errors?: Array<{ message: string }>;
}
/**
* Make a GraphQL request to GitHub API
*/
async function githubGraphQL<T>(
token: string,
query: string,
variables: Record<string, unknown> = {}
): Promise<T> {
const response = await fetch("https://api.github.com/graphql", {
method: "POST",
headers: {
"Authorization": `Bearer ${token}`,
"Content-Type": "application/json",
"User-Agent": "Auto-Claude-UI",
},
body: JSON.stringify({ query, variables }),
});
if (!response.ok) {
// Log detailed error for debugging, throw generic message for safety
console.error(`GitHub GraphQL HTTP error: ${response.status} ${response.statusText}`);
throw new Error("Failed to connect to GitHub API");
}
const result = await response.json() as T & { errors?: Array<{ message: string }> };
// Check for GraphQL-level errors
if (result.errors && result.errors.length > 0) {
// Log detailed errors for debugging, throw generic message for safety
console.error(`GitHub GraphQL errors: ${result.errors.map(e => e.message).join(", ")}`);
throw new Error("GitHub API request failed");
}
return result;
}
/**
* GraphQL query to fetch PRs with diff stats
*/
const LIST_PRS_QUERY = `
query($owner: String!, $repo: String!, $first: Int!, $after: String) {
repository(owner: $owner, name: $repo) {
pullRequests(states: OPEN, first: $first, after: $after, orderBy: {field: UPDATED_AT, direction: DESC}) {
pageInfo { hasNextPage endCursor }
nodes {
number
title
body
state
author { login }
headRefName
baseRefName
additions
deletions
changedFiles
assignees(first: 10) { nodes { login } }
createdAt
updatedAt
url
}
}
}
}
`;
/**
* Sanitize network data before writing to file
* Removes potentially dangerous characters and limits length
@@ -382,6 +481,14 @@ export interface PRData {
htmlUrl: string;
}
/**
* PR list result with pagination info
*/
export interface PRListResult {
prs: PRData[];
hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
}
/**
* PR review progress status
*/
@@ -1231,66 +1338,78 @@ async function runPRReview(
export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void {
debugLog("Registering PR handlers");
// List open PRs with pagination support
// List open PRs - fetches up to 100 open PRs at once, returns hasNextPage from API
ipcMain.handle(
IPC_CHANNELS.GITHUB_PR_LIST,
async (_, projectId: string, page: number = 1): Promise<PRData[]> => {
debugLog("listPRs handler called", { projectId, page });
async (_, projectId: string): Promise<PRListResult> => {
debugLog("listPRs handler called", { projectId });
const result = await withProjectOrNull(projectId, async (project) => {
const config = getGitHubConfig(project);
if (!config) {
debugLog("No GitHub config found for project");
return [];
return { prs: [], hasNextPage: false };
}
try {
// Use pagination: per_page=100 (GitHub max), page=1,2,3...
const prs = (await githubFetch(
config.token,
`/repos/${config.repo}/pulls?state=open&per_page=100&page=${page}`
)) as Array<{
number: number;
title: string;
body?: string;
state: string;
user: { login: string };
head: { ref: string };
base: { ref: string };
additions: number;
deletions: number;
changed_files: number;
assignees?: Array<{ login: string }>;
created_at: string;
updated_at: string;
html_url: string;
}>;
// Parse owner/repo from config - must be exactly "owner/repo" format
const normalizedRepo = normalizeRepoReference(config.repo);
const repoParts = normalizedRepo.split("/");
if (repoParts.length !== 2 || !repoParts[0] || !repoParts[1]) {
debugLog("Invalid repo format - expected 'owner/repo'", { repo: config.repo, normalized: normalizedRepo });
return { prs: [], hasNextPage: false };
}
const [owner, repo] = repoParts;
debugLog("Fetched PRs", { count: prs.length, page, samplePr: prs[0] });
return prs.map((pr) => ({
number: pr.number,
title: pr.title,
body: pr.body ?? "",
state: pr.state,
author: { login: pr.user.login },
headRefName: pr.head.ref,
baseRefName: pr.base.ref,
additions: pr.additions ?? 0,
deletions: pr.deletions ?? 0,
changedFiles: pr.changed_files ?? 0,
assignees: pr.assignees?.map((a: { login: string }) => ({ login: a.login })) ?? [],
files: [],
createdAt: pr.created_at,
updatedAt: pr.updated_at,
htmlUrl: pr.html_url,
}));
// Use GraphQL API to get PRs with diff stats (REST list endpoint doesn't include them)
// Fetches up to 100 open PRs (GitHub GraphQL max per request)
const response = await githubGraphQL<GraphQLPRListResponse>(
config.token,
LIST_PRS_QUERY,
{
owner,
repo,
first: 100, // GitHub GraphQL max is 100
after: null, // Start from beginning
}
);
// Handle case where repository doesn't exist or user lacks access
if (!response.data.repository) {
debugLog("Repository not found or access denied", { owner, repo });
return { prs: [], hasNextPage: false };
}
const { nodes: prNodes, pageInfo } = response.data.repository.pullRequests;
debugLog("Fetched PRs via GraphQL", { count: prNodes.length, hasNextPage: pageInfo.hasNextPage });
return {
prs: prNodes.map((pr) => ({
number: pr.number,
title: pr.title,
body: pr.body ?? "",
state: pr.state.toLowerCase(),
author: { login: pr.author?.login ?? "unknown" },
headRefName: pr.headRefName,
baseRefName: pr.baseRefName,
additions: pr.additions,
deletions: pr.deletions,
changedFiles: pr.changedFiles,
assignees: pr.assignees.nodes.map((a) => ({ login: a.login })),
files: [],
createdAt: pr.createdAt,
updatedAt: pr.updatedAt,
htmlUrl: pr.url,
})),
hasNextPage: pageInfo.hasNextPage,
};
} catch (error) {
debugLog("Failed to fetch PRs", {
error: error instanceof Error ? error.message : error,
});
return [];
return { prs: [], hasNextPage: false };
}
});
return result ?? [];
return result ?? { prs: [], hasNextPage: false };
}
);
@@ -267,8 +267,8 @@ export interface GitHubAPI {
callback: (projectId: string, error: { error: string }) => void
) => IpcListenerCleanup;
// PR operations
listPRs: (projectId: string, page?: number) => Promise<PRData[]>;
// PR operations (fetches up to 100 open PRs at once - GitHub GraphQL limit)
listPRs: (projectId: string) => Promise<PRListResult>;
getPR: (projectId: string, prNumber: number) => Promise<PRData | null>;
runPRReview: (projectId: string, prNumber: number) => void;
cancelPRReview: (projectId: string, prNumber: number) => Promise<boolean>;
@@ -332,6 +332,14 @@ export interface PRData {
htmlUrl: string;
}
/**
* PR list result with pagination info
*/
export interface PRListResult {
prs: PRData[];
hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
}
/**
* PR review finding
*/
@@ -663,8 +671,9 @@ export const createGitHubAPI = (): GitHubAPI => ({
createIpcListener(IPC_CHANNELS.GITHUB_AUTOFIX_ANALYZE_PREVIEW_ERROR, callback),
// PR operations
listPRs: (projectId: string, page: number = 1): Promise<PRData[]> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId, page),
// Fetches up to 100 open PRs at once (GitHub GraphQL limit)
listPRs: (projectId: string): Promise<PRListResult> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId),
getPR: (projectId: string, prNumber: number): Promise<PRData | null> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_GET, projectId, prNumber),
@@ -58,7 +58,6 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
const {
prs,
isLoading,
isLoadingMore,
isLoadingPRDetails,
error,
selectedPRNumber,
@@ -79,7 +78,6 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
assignPR,
markReviewPosted,
refresh,
loadMore,
isConnected,
repoFullName,
getReviewStateForPR,
@@ -234,12 +232,10 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
prs={filteredPRs}
selectedPRNumber={selectedPRNumber}
isLoading={isLoading}
isLoadingMore={isLoadingMore}
hasMore={hasMore}
error={error}
getReviewStateForPR={getReviewStateForPR}
onSelectPR={selectPR}
onLoadMore={loadMore}
/>
</div>
}
@@ -1,5 +1,4 @@
import { useRef, useEffect, useCallback, useState } from 'react';
import { GitPullRequest, User, Clock, FileDiff, Loader2 } from 'lucide-react';
import { GitPullRequest, User, Clock, FileDiff } from 'lucide-react';
import { ScrollArea } from '../../ui/scroll-area';
import { Badge } from '../../ui/badge';
import { cn } from '../../../lib/utils';
@@ -167,12 +166,10 @@ interface PRListProps {
prs: PRData[];
selectedPRNumber: number | null;
isLoading: boolean;
isLoadingMore: boolean;
hasMore: boolean;
hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist
error: string | null;
getReviewStateForPR: (prNumber: number) => PRReviewInfo | null;
onSelectPR: (prNumber: number) => void;
onLoadMore: () => void;
}
function formatDate(dateString: string): string {
@@ -199,41 +196,12 @@ export function PRList({
prs,
selectedPRNumber,
isLoading,
isLoadingMore,
hasMore,
error,
getReviewStateForPR,
onSelectPR,
onLoadMore
}: PRListProps) {
const { t } = useTranslation('common');
const loadMoreTriggerRef = useRef<HTMLDivElement>(null);
const [viewportElement, setViewportElement] = useState<HTMLDivElement | null>(null);
// Intersection Observer for infinite scroll
const handleIntersection = useCallback((entries: IntersectionObserverEntry[]) => {
const [entry] = entries;
if (entry.isIntersecting && hasMore && !isLoadingMore && !isLoading) {
onLoadMore();
}
}, [hasMore, isLoadingMore, isLoading, onLoadMore]);
useEffect(() => {
const trigger = loadMoreTriggerRef.current;
if (!trigger || !viewportElement) return;
const observer = new IntersectionObserver(handleIntersection, {
root: viewportElement,
rootMargin: '100px',
threshold: 0
});
observer.observe(trigger);
return () => {
observer.disconnect();
};
}, [handleIntersection, onLoadMore, viewportElement]);
if (isLoading && prs.length === 0) {
return (
@@ -268,7 +236,7 @@ export function PRList({
}
return (
<ScrollArea className="flex-1" onViewportRef={setViewportElement}>
<ScrollArea className="flex-1">
<div className="divide-y divide-border">
{prs.map((pr) => {
const reviewState = getReviewStateForPR(pr.number);
@@ -338,23 +306,14 @@ export function PRList({
);
})}
{/* Load more trigger / Loading indicator */}
<div ref={loadMoreTriggerRef} className="py-4 flex justify-center">
{isLoadingMore ? (
<div className="flex items-center gap-2 text-muted-foreground">
<Loader2 className="h-4 w-4 animate-spin" />
<span className="text-sm">{t('prReview.loadingMore')}</span>
</div>
) : hasMore ? (
{/* Status indicator */}
{prs.length > 0 && (
<div className="py-4 flex justify-center">
<span className="text-xs text-muted-foreground opacity-50">
{t('prReview.scrollForMore')}
{hasMore ? t('prReview.maxPRsShown') : t('prReview.allPRsLoaded')}
</span>
) : prs.length > 0 ? (
<span className="text-xs text-muted-foreground opacity-50">
{t('prReview.allPRsLoaded')}
</span>
) : null}
</div>
</div>
)}
</div>
</ScrollArea>
);
@@ -23,7 +23,6 @@ interface UseGitHubPRsOptions {
interface UseGitHubPRsResult {
prs: PRData[];
isLoading: boolean;
isLoadingMore: boolean;
isLoadingPRDetails: boolean; // Loading full PR details including files
error: string | null;
selectedPR: PRData | null;
@@ -36,10 +35,9 @@ interface UseGitHubPRsResult {
isConnected: boolean;
repoFullName: string | null;
activePRReviews: number[]; // PR numbers currently being reviewed
hasMore: boolean; // Whether there are more PRs to load
hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist
selectPR: (prNumber: number | null) => void;
refresh: () => Promise<void>;
loadMore: () => Promise<void>;
runReview: (prNumber: number) => void;
runFollowupReview: (prNumber: number) => void;
checkNewCommits: (prNumber: number) => Promise<NewCommitsCheck>;
@@ -71,15 +69,13 @@ export function useGitHubPRs(
const { isActive = true } = options;
const [prs, setPrs] = useState<PRData[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [isLoadingMore, setIsLoadingMore] = useState(false);
const [isLoadingPRDetails, setIsLoadingPRDetails] = useState(false);
const [error, setError] = useState<string | null>(null);
const [selectedPRNumber, setSelectedPRNumber] = useState<number | null>(null);
const [selectedPRDetails, setSelectedPRDetails] = useState<PRData | null>(null);
const [isConnected, setIsConnected] = useState(false);
const [repoFullName, setRepoFullName] = useState<string | null>(null);
const [currentPage, setCurrentPage] = useState(1);
const [hasMore, setHasMore] = useState(true);
const [hasMore, setHasMore] = useState(false);
// Track previous isActive state to detect tab navigation
const wasActiveRef = useRef(isActive);
@@ -144,14 +140,10 @@ export function useGitHubPRs(
// Check connection and fetch PRs
const fetchPRs = useCallback(
async (page: number = 1, append: boolean = false) => {
async () => {
if (!projectId) return;
if (append) {
setIsLoadingMore(true);
} else {
setIsLoading(true);
}
setIsLoading(true);
setError(null);
try {
@@ -162,27 +154,16 @@ export function useGitHubPRs(
setRepoFullName(connectionResult.data.repoFullName || null);
if (connectionResult.data.connected) {
// Fetch PRs with pagination
const result = await window.electronAPI.github.listPRs(projectId, page);
// Fetch PRs (returns up to 100 open PRs at once - GitHub GraphQL limit)
const result = await window.electronAPI.github.listPRs(projectId);
if (result) {
// Check if there are more PRs to load (GitHub returns up to 100 per page)
setHasMore(result.length === 100);
setCurrentPage(page);
if (append) {
// Append to existing PRs, deduplicating by PR number
setPrs((prevPrs) => {
const existingNumbers = new Set(prevPrs.map((pr) => pr.number));
const newPrs = result.filter((pr) => !existingNumbers.has(pr.number));
return [...prevPrs, ...newPrs];
});
} else {
setPrs(result);
}
// Use hasNextPage from API to determine if more PRs exist
setHasMore(result.hasNextPage);
setPrs(result.prs);
// Batch preload review results for PRs not in store (single IPC call)
// Skip PRs that are currently being reviewed - their state is managed by IPC listeners
const prsNeedingPreload = result.filter((pr) => {
const prsNeedingPreload = result.prs.filter((pr) => {
const existingState = getPRReviewState(projectId, pr.number);
return !existingState?.result && !existingState?.isReviewing;
});
@@ -218,7 +199,6 @@ export function useGitHubPRs(
setIsConnected(false);
} finally {
setIsLoading(false);
setIsLoadingMore(false);
}
},
[projectId, getPRReviewState]
@@ -228,7 +208,7 @@ export function useGitHubPRs(
useEffect(() => {
if (projectId && !hasLoadedRef.current) {
hasLoadedRef.current = true;
fetchPRs(1, false);
fetchPRs();
}
}, [projectId, fetchPRs]);
@@ -236,19 +216,15 @@ export function useGitHubPRs(
useEffect(() => {
// Only refresh if transitioning from inactive to active AND we've loaded before
if (isActive && !wasActiveRef.current && hasLoadedRef.current) {
// Reset to first page and refresh
setCurrentPage(1);
setHasMore(true);
fetchPRs(1, false);
fetchPRs();
}
wasActiveRef.current = isActive;
}, [isActive, fetchPRs]);
// Reset pagination and selected PR when project changes
// Reset state and selected PR when project changes
useEffect(() => {
hasLoadedRef.current = false;
setCurrentPage(1);
setHasMore(true);
setHasMore(false);
setPrs([]);
setSelectedPRNumber(null);
setSelectedPRDetails(null);
@@ -398,16 +374,9 @@ export function useGitHubPRs(
);
const refresh = useCallback(async () => {
setCurrentPage(1);
setHasMore(true);
await fetchPRs(1, false);
await fetchPRs();
}, [fetchPRs]);
const loadMore = useCallback(async () => {
if (!hasMore || isLoadingMore || isLoading) return;
await fetchPRs(currentPage + 1, true);
}, [fetchPRs, hasMore, isLoadingMore, isLoading, currentPage]);
const runReview = useCallback(
(prNumber: number) => {
if (!projectId) return;
@@ -596,7 +565,6 @@ export function useGitHubPRs(
return {
prs,
isLoading,
isLoadingMore,
isLoadingPRDetails,
error,
selectedPR,
@@ -612,7 +580,6 @@ export function useGitHubPRs(
hasMore,
selectPR,
refresh,
loadMore,
runReview,
runFollowupReview,
checkNewCommits,
@@ -202,7 +202,7 @@ const browserMockAPI: ElectronAPI = {
onAutoFixProgress: () => () => {},
onAutoFixComplete: () => () => {},
onAutoFixError: () => () => {},
listPRs: async () => [],
listPRs: async () => ({ prs: [], hasNextPage: false }),
getPR: async () => null,
runPRReview: () => {},
cancelPRReview: async () => true,
@@ -339,9 +339,8 @@
"updatingBranch": "Updating...",
"branchUpdated": "Branch updated",
"branchUpdateFailed": "Failed to update branch",
"loadingMore": "Loading more PRs...",
"scrollForMore": "Scroll for more",
"allPRsLoaded": "All PRs loaded",
"maxPRsShown": "Showing first 100 PRs",
"workflowsAwaitingApproval": "{{count}} Workflow Awaiting Approval",
"workflowsAwaitingApproval_plural": "{{count}} Workflows Awaiting Approval",
"blockedByWorkflows": "Blocked",
@@ -348,9 +348,8 @@
"updatingBranch": "Mise à jour...",
"branchUpdated": "Branche mise à jour",
"branchUpdateFailed": "Échec de la mise à jour de la branche",
"loadingMore": "Chargement des PRs...",
"scrollForMore": "Défiler pour plus",
"allPRsLoaded": "Tous les PRs chargés",
"maxPRsShown": "Affichage des 100 premières PRs",
"workflowsAwaitingApproval": "{{count}} workflow en attente d'approbation",
"workflowsAwaitingApproval_plural": "{{count}} workflows en attente d'approbation",
"blockedByWorkflows": "Bloqué",