auto-claude: 182-implement-pagination-and-filtering-for-github-pr-l (#1654)

* auto-claude: subtask-1-1 - Add GITHUB_PR_LIST_MORE IPC channel constant Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-1-2 - Add listMorePRs IPC handler for pagination - Add GITHUB_PR_LIST_MORE handler that accepts cursor parameter - Update PRListResult interface to include endCursor field - Update existing GITHUB_PR_LIST handler to also return endCursor - Both handlers use GraphQL pagination with cursor-based navigation Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-1-3 - Update PRListResult type to include endCursor field Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-1-4 - Add listMorePRs method to GitHubAPI interface Add listMorePRs method for cursor-based pagination to the GitHubAPI interface and createGitHubAPI implementation in preload. Also update browser-mock.ts to include the new method for type consistency. Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-2-1 - Add sortBy option to PRFilterState interface and DEFAULT_FILTERS - Added PRSortOption type with 'newest' | 'oldest' | 'largest' options - Added sortBy field to PRFilterState interface - Set default sortBy to 'newest' in DEFAULT_FILTERS Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-2-2 - Add sorting logic to filteredPRs useMemo in usePRF * auto-claude: subtask-2-4 - Add loadMore function, endCursor state, and isLoadingMore state - Add isLoadingMore state to track pagination loading - Add endCursor state to track pagination cursor - Add loadMore function for cursor-based pagination - Update UseGitHubPRsResult interface with new properties - Store endCursor from fetchPRs API response - Reset endCursor when project changes - Batch preload review results for newly loaded PRs Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-3-1 - Add sort dropdown to PRFilterBar using FilterDropdown - Add SortDropdown component with single-select behavior for sorting PRs - Add SORT_OPTIONS constant with newest/oldest/largest options - Add onSortChange prop to PRFilterBarProps interface - Import ArrowUpDown, Clock, FileCode icons from lucide-react - Import PRSortOption type from usePRFiltering hook - Update GitHubPRs.tsx to pass setSortBy as onSortChange prop - Add i18n translations for sort labels (en/fr) Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-3-2 - Add onLoadMore and isLoadingMore props to PRList component Add pagination props to PRListProps interface: - onLoadMore: Optional callback to load more PRs when hasMore is true - isLoadingMore: Optional boolean to track loading state for pagination Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-3-3 - Replace status indicator text with Load More button - Add Load More button component to PRList when hasMore is true - Show loading spinner with "Loading..." text when isLoadingMore is true - Keep "All PRs loaded" text when all PRs are displayed - Add prReview.loadMore and prReview.loadingMore translation keys - Import Button and Loader2 components Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-3-4 - Wire up new props in GitHubPRs.tsx parent component - Add loadMore and isLoadingMore to useGitHubPRs destructuring - Pass loadMore as onLoadMore prop to PRList component - Pass isLoadingMore to PRList component for loading state - setSortBy already wired to PRFilterBar's onSortChange Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-4-1 - Add English translation keys for sortBy, sortNewest, sortOldest, sortLargest, loadMore, loadingMore Co-Authored-By: Claude Opus 4.5 <[email protected]> * auto-claude: subtask-4-2 - Add French translation keys for sortBy, sortNewest Adds French translations for pagination and sorting UI elements: - sort.sortBy: "Trier par" - sort.sortNewest: "Plus récent" - sort.sortOldest: "Plus ancien" - sort.sortLargest: "Plus grand" - pagination.loadMore: "Charger plus" - pagination.loadingMore: "Chargement..." Co-Authored-By: Claude Opus 4.5 <[email protected]> * Fix ruff formatting and Windows dataclass import error - Apply ruff formatting to parallel_orchestrator_reviewer.py (3 long lines) - Apply ruff formatting to pydantic_models.py (Field on single line) - Fix Windows test collection error: register module in sys.modules before exec_module so dataclass decorator can find it Co-Authored-By: Claude Opus 4.5 <[email protected]> * Fix PR review findings: dedup mapping, race conditions, unused i18n keys - Extract shared mapGraphQLPRToData helper to eliminate duplicated PR mapping logic between listPRs and listMorePRs handlers - Add staleness checks to loadMore using generation counter and projectId ref to prevent race conditions with refresh and project switching - Reset isLoadingMore on project change to prevent stuck loading state - Remove unused common.sort.* and common.pagination.* translation keys from en/fr locale files (code uses prReview.* keys instead) - Align endCursor type to string | null in preload PRListResult - Preserve sortBy preference when clearing filters for consistency with hasActiveFilters Co-Authored-By: Claude Opus 4.5 <[email protected]> * Fix PR review findings: dedup PR handlers, stable sort order - Extract fetchPRsFromGraphQL helper to deduplicate listPRs/listMorePRs handlers - Add secondary sort key (createdAt) to 'largest' sort for stable ordering Co-Authored-By: Claude Opus 4.5 <[email protected]> * Fix PR review findings: deduplication and keyboard navigation - Add deduplication by PR number when appending paginated PRs to prevent duplicates if a PR shifts position between pagination requests - Add keyboard navigation to SortDropdown (ArrowUp/Down, Enter, Space, Escape) matching the pattern used in FilterDropdown for accessibility consistency Co-Authored-By: Claude Opus 4.5 <[email protected]> * Fix follow-up review findings: pagination, sort, keyboard UX - Preserve pagination state on failure response to allow retry - Pre-compute timestamps before sorting to avoid Date object creation - Add scrollIntoView for keyboard-focused items in FilterDropdown - Focus current selection when SortDropdown opens for better keyboard UX Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]> Co-authored-by: Test User <[email protected]>
2026-02-09 12:31:35 +02:00
parent 4e6d2b3fad
commit fd263b424f
8 changed files with 205 additions and 83 deletions
@@ -225,6 +225,23 @@ query($owner: String!, $repo: String!, $first: Int!, $after: String) {
 * GraphQL response type for PR list query
 * Note: repository can be null if the repo doesn't exist or user lacks access
 */
+interface GraphQLPRNode {
+  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;
+}
+
 interface GraphQLPRListResponse {
  data: {
    repository: {
@@ -233,28 +250,37 @@ interface GraphQLPRListResponse {
          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;
-        }>;
+        nodes: GraphQLPRNode[];
      };
    } | null;
  };
  errors?: Array<{ message: string }>;
 }

+/**
+ * Maps a GraphQL PR node to the frontend PRData format.
+ * Shared between listPRs and listMorePRs handlers.
+ */
+function mapGraphQLPRToData(pr: GraphQLPRNode): PRData {
+  return {
+    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,
+  };
+}
+
 /**
 * Make a GraphQL request to GitHub API
 */
@@ -724,6 +750,7 @@ export interface PRData {
 export interface PRListResult {
  prs: PRData[];
  hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
+  endCursor?: string | null; // Cursor for fetching next page (null if no more pages)
 }

 /**
@@ -1821,7 +1848,7 @@ async function fetchPRsFromGraphQL(
 export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void {
  debugLog("Registering PR handlers");

-  // List open PRs - fetches up to 100 open PRs at once, returns hasNextPage from API
+  // List open PRs - fetches up to 100 open PRs at once, returns hasNextPage and endCursor from API
  ipcMain.handle(
    IPC_CHANNELS.GITHUB_PR_LIST,
    async (_, projectId: string): Promise<PRListResult> => {
@@ -1830,70 +1857,28 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
        const config = getGitHubConfig(project);
        if (!config) {
          debugLog("No GitHub config found for project");
-          return { prs: [], hasNextPage: false };
-        }
-
-        try {
-          // 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;
-
-          // 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 { prs: [], hasNextPage: false };
+          return { prs: [], hasNextPage: false, endCursor: null };
        }
        return fetchPRsFromGraphQL(config, null, "initial");
      });
-      return result ?? { prs: [], hasNextPage: false };
+      return result ?? { prs: [], hasNextPage: false, endCursor: null };
+    }
+  );
+
+  // Load more PRs (pagination) - fetches next page of PRs using cursor
+  ipcMain.handle(
+    IPC_CHANNELS.GITHUB_PR_LIST_MORE,
+    async (_, projectId: string, cursor: string): Promise<PRListResult> => {
+      debugLog("listMorePRs handler called", { projectId, cursor });
+      const result = await withProjectOrNull(projectId, async (project) => {
+        const config = getGitHubConfig(project);
+        if (!config) {
+          debugLog("No GitHub config found for project");
+          return { prs: [], hasNextPage: false, endCursor: null };
+        }
+        return fetchPRsFromGraphQL(config, cursor, "pagination");
+      });
+      return result ?? { prs: [], hasNextPage: false, endCursor: null };
    }
  );

@@ -271,6 +271,8 @@ export interface GitHubAPI {

  // PR operations (fetches up to 100 open PRs at once - GitHub GraphQL limit)
  listPRs: (projectId: string) => Promise<PRListResult>;
+  /** Load more PRs using cursor-based pagination */
+  listMorePRs: (projectId: string, cursor: string) => Promise<PRListResult>;
  getPR: (projectId: string, prNumber: number) => Promise<PRData | null>;
  runPRReview: (projectId: string, prNumber: number) => void;
  cancelPRReview: (projectId: string, prNumber: number) => Promise<boolean>;
@@ -357,6 +359,7 @@ export interface PRData {
 export interface PRListResult {
  prs: PRData[];
  hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
+  endCursor?: string | null; // Cursor for fetching next page (null if no more pages)
 }

 /**
@@ -694,6 +697,10 @@ export const createGitHubAPI = (): GitHubAPI => ({
  listPRs: (projectId: string): Promise<PRListResult> =>
    invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId),

+  // Load more PRs using cursor-based pagination
+  listMorePRs: (projectId: string, cursor: string): Promise<PRListResult> =>
+    invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST_MORE, projectId, cursor),
+
  getPR: (projectId: string, prNumber: number): Promise<PRData | null> =>
    invokeIpc(IPC_CHANNELS.GITHUB_PR_GET, projectId, prNumber),

@@ -58,6 +58,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
  const {
    prs,
    isLoading,
+    isLoadingMore,
    isLoadingPRDetails,
    error,
    selectedPRNumber,
@@ -78,6 +79,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
    assignPR,
    markReviewPosted,
    refresh,
+    loadMore,
    isConnected,
    repoFullName,
    getReviewStateForPR,
@@ -252,6 +254,8 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
              error={error}
              getReviewStateForPR={getReviewStateForPR}
              onSelectPR={selectPR}
+              onLoadMore={loadMore}
+              isLoadingMore={isLoadingMore}
            />
          </div>
        }
@@ -1,4 +1,4 @@
-import { GitPullRequest, User, Clock, FileDiff } from 'lucide-react';
+import { GitPullRequest, User, Clock, FileDiff, Loader2 } from 'lucide-react';
 import { ScrollArea } from '../../ui/scroll-area';
 import { Badge } from '../../ui/badge';
 import { Button } from '../../ui/button';
@@ -179,6 +179,10 @@ interface PRListProps {
  error: string | null;
  getReviewStateForPR: (prNumber: number) => PRReviewInfo | null;
  onSelectPR: (prNumber: number) => void;
+  /** Callback to load more PRs when hasMore is true */
+  onLoadMore?: () => void;
+  /** Whether additional PRs are currently being loaded */
+  isLoadingMore?: boolean;
 }

 function formatDate(dateString: string): string {
@@ -209,6 +213,8 @@ export function PRList({
  error,
  getReviewStateForPR,
  onSelectPR,
+  onLoadMore,
+  isLoadingMore,
 }: PRListProps) {
  const { t } = useTranslation('common');

@@ -322,12 +328,30 @@ export function PRList({
          );
        })}

-        {/* Status indicator */}
+        {/* Status indicator / Load More button */}
        {prs.length > 0 && (
          <div className="py-4 flex justify-center">
-            <span className="text-xs text-muted-foreground opacity-50">
-              {hasMore ? t('prReview.maxPRsShown') : t('prReview.allPRsLoaded')}
-            </span>
+            {hasMore && onLoadMore ? (
+              <Button
+                variant="outline"
+                size="sm"
+                onClick={onLoadMore}
+                disabled={isLoadingMore}
+              >
+                {isLoadingMore ? (
+                  <>
+                    <Loader2 className="mr-2 h-4 w-4 animate-spin" />
+                    {t('prReview.loadingMore')}
+                  </>
+                ) : (
+                  t('prReview.loadMore')
+                )}
+              </Button>
+            ) : (
+              <span className="text-xs text-muted-foreground opacity-50">
+                {t('prReview.allPRsLoaded')}
+              </span>
+            )}
          </div>
        )}
      </div>
@@ -23,6 +23,7 @@ interface UseGitHubPRsOptions {
 interface UseGitHubPRsResult {
  prs: PRData[];
  isLoading: boolean;
+  isLoadingMore: boolean; // Loading additional PRs via pagination
  isLoadingPRDetails: boolean; // Loading full PR details including files
  error: string | null;
  selectedPR: PRData | null;
@@ -38,6 +39,7 @@ interface UseGitHubPRsResult {
  hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist
  selectPR: (prNumber: number | null) => void;
  refresh: () => Promise<void>;
+  loadMore: () => Promise<void>; // Load next page of PRs
  runReview: (prNumber: number) => void;
  runFollowupReview: (prNumber: number) => void;
  checkNewCommits: (prNumber: number) => Promise<NewCommitsCheck>;
@@ -76,6 +78,8 @@ export function useGitHubPRs(
  const [isConnected, setIsConnected] = useState(false);
  const [repoFullName, setRepoFullName] = useState<string | null>(null);
  const [hasMore, setHasMore] = useState(false);
+  const [isLoadingMore, setIsLoadingMore] = useState(false);
+  const [endCursor, setEndCursor] = useState<string | null>(null);

  // Track previous isActive state to detect tab navigation
  const wasActiveRef = useRef(isActive);
@@ -158,6 +162,9 @@ export function useGitHubPRs(
    async () => {
      if (!projectId) return;

+      // Increment generation to invalidate any in-flight loadMore requests
+      fetchGenerationRef.current += 1;
+
      setIsLoading(true);
      setError(null);

@@ -174,6 +181,8 @@ export function useGitHubPRs(
            if (result) {
              // Use hasNextPage from API to determine if more PRs exist
              setHasMore(result.hasNextPage);
+              // Store endCursor for pagination
+              setEndCursor(result.endCursor ?? null);
              setPrs(result.prs);

              // Batch preload review results for PRs not in store (single IPC call)
@@ -242,6 +251,7 @@ export function useGitHubPRs(
    fetchGenerationRef.current += 1;
    hasLoadedRef.current = false;
    setHasMore(false);
+    setEndCursor(null);
    setPrs([]);
    setSelectedPRNumber(null);
    setSelectedPRDetails(null);
@@ -433,6 +443,91 @@ export function useGitHubPRs(
    await fetchPRs();
  }, [fetchPRs]);

+  // Load more PRs using cursor-based pagination
+  const loadMore = useCallback(async () => {
+    if (!projectId || !endCursor || !hasMore || isLoadingMore) return;
+
+    // Capture current state for staleness checks
+    const requestProjectId = projectId;
+    const requestGeneration = fetchGenerationRef.current;
+
+    setIsLoadingMore(true);
+    setError(null);
+
+    try {
+      const result = await window.electronAPI.github.listMorePRs(projectId, endCursor);
+
+      // Discard response if project changed or a refresh happened while loading
+      if (
+        requestProjectId !== currentProjectIdRef.current ||
+        requestGeneration !== fetchGenerationRef.current
+      ) {
+        return;
+      }
+
+      if (result) {
+        // Check if this is a failure response (empty result with no next page)
+        // In this case, preserve existing pagination state to allow retry
+        const isFailureResponse = result.prs.length === 0 && !result.hasNextPage && !result.endCursor;
+
+        if (!isFailureResponse) {
+          // Update pagination state only on successful response
+          setHasMore(result.hasNextPage);
+          setEndCursor(result.endCursor ?? null);
+
+          // Append new PRs to existing list, deduplicating by PR number
+          // (handles edge case where PR shifts position between pagination requests)
+          setPrs((prevPrs) => {
+            const existingNumbers = new Set(prevPrs.map((pr) => pr.number));
+            const newPrs = result.prs.filter((pr) => !existingNumbers.has(pr.number));
+            return [...prevPrs, ...newPrs];
+          });
+        }
+
+        // Batch preload review results for new PRs not in store
+        const prsNeedingPreload = result.prs.filter((pr) => {
+          const existingState = getPRReviewState(requestProjectId, pr.number);
+          return !existingState?.result && !existingState?.isReviewing;
+        });
+
+        if (prsNeedingPreload.length > 0) {
+          const prNumbers = prsNeedingPreload.map((pr) => pr.number);
+          const batchReviews = await window.electronAPI.github.getPRReviewsBatch(
+            requestProjectId,
+            prNumbers
+          );
+
+          // Check staleness again after async batch fetch
+          if (
+            requestProjectId !== currentProjectIdRef.current ||
+            requestGeneration !== fetchGenerationRef.current
+          ) {
+            return;
+          }
+
+          // Update store with loaded results
+          for (const reviewResult of Object.values(batchReviews)) {
+            if (reviewResult) {
+              usePRReviewStore.getState().setPRReviewResult(requestProjectId, reviewResult, {
+                preserveNewCommitsCheck: true,
+              });
+            }
+          }
+        }
+      }
+    } catch (err) {
+      // Only show error if still relevant
+      if (
+        requestProjectId === currentProjectIdRef.current &&
+        requestGeneration === fetchGenerationRef.current
+      ) {
+        setError(err instanceof Error ? err.message : "Failed to load more PRs");
+      }
+    } finally {
+      setIsLoadingMore(false);
+    }
+  }, [projectId, endCursor, hasMore, isLoadingMore, getPRReviewState]);
+
  const runReview = useCallback(
    (prNumber: number) => {
      if (!projectId) return;
@@ -623,6 +718,7 @@ export function useGitHubPRs(
  return {
    prs,
    isLoading,
+    isLoadingMore,
    isLoadingPRDetails,
    error,
    selectedPR,
@@ -638,6 +734,7 @@ export function useGitHubPRs(
    hasMore,
    selectPR,
    refresh,
+    loadMore,
    runReview,
    runFollowupReview,
    checkNewCommits,
@@ -203,6 +203,7 @@ const browserMockAPI: ElectronAPI = {
    onAutoFixComplete: () => () => {},
    onAutoFixError: () => () => {},
    listPRs: async () => ({ prs: [], hasNextPage: false }),
+    listMorePRs: async () => ({ prs: [], hasNextPage: false }),
    getPR: async () => null,
    runPRReview: () => {},
    cancelPRReview: async () => true,
@@ -367,6 +367,8 @@
    "branchUpdateFailed": "Failed to update branch",
    "allPRsLoaded": "All PRs loaded",
    "maxPRsShown": "Showing first 100 PRs",
+    "loadMore": "Load More",
+    "loadingMore": "Loading...",
    "workflowsAwaitingApproval": "{{count}} Workflow Awaiting Approval",
    "workflowsAwaitingApproval_plural": "{{count}} Workflows Awaiting Approval",
    "blockedByWorkflows": "Blocked",
@@ -376,6 +376,8 @@
    "branchUpdateFailed": "Échec de la mise à jour de la branche",
    "allPRsLoaded": "Tous les PRs chargés",
    "maxPRsShown": "Affichage des 100 premières PRs",
+    "loadMore": "Charger plus",
+    "loadingMore": "Chargement...",
    "workflowsAwaitingApproval": "{{count}} workflow en attente d'approbation",
    "workflowsAwaitingApproval_plural": "{{count}} workflows en attente d'approbation",
    "blockedByWorkflows": "Bloqué",