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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update PRListResult type to include endCursor field

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

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add English translation keys for sortBy, sortNewest, sortOldest, sortLargest, loadMore, loadingMore

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

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Test User <test@example.com>
This commit is contained in:
Andy
2026-02-04 14:06:49 +01:00
committed by GitHub
parent d9cd300fee
commit b4e6b2fe43
11 changed files with 493 additions and 89 deletions
@@ -40,6 +40,23 @@ import {
* GraphQL response type for PR list query * GraphQL response type for PR list query
* Note: repository can be null if the repo doesn't exist or user lacks access * 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 { interface GraphQLPRListResponse {
data: { data: {
repository: { repository: {
@@ -48,28 +65,37 @@ interface GraphQLPRListResponse {
hasNextPage: boolean; hasNextPage: boolean;
endCursor: string | null; endCursor: string | null;
}; };
nodes: Array<{ nodes: 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;
}>;
}; };
} | null; } | null;
}; };
errors?: Array<{ message: string }>; 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 * Make a GraphQL request to GitHub API
*/ */
@@ -487,6 +513,7 @@ export interface PRData {
export interface PRListResult { export interface PRListResult {
prs: PRData[]; prs: PRData[];
hasNextPage: boolean; // True if more PRs exist beyond the 100 limit hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
endCursor?: string | null; // Cursor for fetching next page (null if no more pages)
} }
/** /**
@@ -1347,13 +1374,75 @@ async function runPRReview(
} }
} }
/**
* Shared helper to fetch PRs via GraphQL API.
* Used by both listPRs and listMorePRs handlers to avoid code duplication.
*/
async function fetchPRsFromGraphQL(
config: { token: string; repo: string },
cursor: string | null,
debugContext: string
): Promise<PRListResult> {
// 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,
context: debugContext,
});
return { prs: [], hasNextPage: false, endCursor: null };
}
const [owner, repo] = repoParts;
try {
// 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: cursor,
}
);
// Handle case where repository doesn't exist or user lacks access
if (!response.data.repository) {
debugLog("Repository not found or access denied", { owner, repo, context: debugContext });
return { prs: [], hasNextPage: false, endCursor: null };
}
const { nodes: prNodes, pageInfo } = response.data.repository.pullRequests;
debugLog(`Fetched PRs via GraphQL (${debugContext})`, {
count: prNodes.length,
hasNextPage: pageInfo.hasNextPage,
endCursor: pageInfo.endCursor,
});
return {
prs: prNodes.map(mapGraphQLPRToData),
hasNextPage: pageInfo.hasNextPage,
endCursor: pageInfo.endCursor,
};
} catch (error) {
debugLog(`Failed to fetch PRs (${debugContext})`, {
error: error instanceof Error ? error.message : error,
});
return { prs: [], hasNextPage: false, endCursor: null };
}
}
/** /**
* Register PR-related handlers * Register PR-related handlers
*/ */
export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void { export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void {
debugLog("Registering PR handlers"); 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( ipcMain.handle(
IPC_CHANNELS.GITHUB_PR_LIST, IPC_CHANNELS.GITHUB_PR_LIST,
async (_, projectId: string): Promise<PRListResult> => { async (_, projectId: string): Promise<PRListResult> => {
@@ -1362,69 +1451,28 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
const config = getGitHubConfig(project); const config = getGitHubConfig(project);
if (!config) { if (!config) {
debugLog("No GitHub config found for project"); debugLog("No GitHub config found for project");
return { prs: [], hasNextPage: false }; return { prs: [], hasNextPage: false, endCursor: null };
}
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 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 };
} }
); );
@@ -269,6 +269,8 @@ export interface GitHubAPI {
// PR operations (fetches up to 100 open PRs at once - GitHub GraphQL limit) // PR operations (fetches up to 100 open PRs at once - GitHub GraphQL limit)
listPRs: (projectId: string) => Promise<PRListResult>; 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>; getPR: (projectId: string, prNumber: number) => Promise<PRData | null>;
runPRReview: (projectId: string, prNumber: number) => void; runPRReview: (projectId: string, prNumber: number) => void;
cancelPRReview: (projectId: string, prNumber: number) => Promise<boolean>; cancelPRReview: (projectId: string, prNumber: number) => Promise<boolean>;
@@ -338,6 +340,7 @@ export interface PRData {
export interface PRListResult { export interface PRListResult {
prs: PRData[]; prs: PRData[];
hasNextPage: boolean; // True if more PRs exist beyond the 100 limit hasNextPage: boolean; // True if more PRs exist beyond the 100 limit
endCursor?: string | null; // Cursor for fetching next page (null if no more pages)
} }
/** /**
@@ -675,6 +678,10 @@ export const createGitHubAPI = (): GitHubAPI => ({
listPRs: (projectId: string): Promise<PRListResult> => listPRs: (projectId: string): Promise<PRListResult> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId), 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> => getPR: (projectId: string, prNumber: number): Promise<PRData | null> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_GET, projectId, prNumber), invokeIpc(IPC_CHANNELS.GITHUB_PR_GET, projectId, prNumber),
@@ -58,6 +58,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
const { const {
prs, prs,
isLoading, isLoading,
isLoadingMore,
isLoadingPRDetails, isLoadingPRDetails,
error, error,
selectedPRNumber, selectedPRNumber,
@@ -78,6 +79,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
assignPR, assignPR,
markReviewPosted, markReviewPosted,
refresh, refresh,
loadMore,
isConnected, isConnected,
repoFullName, repoFullName,
getReviewStateForPR, getReviewStateForPR,
@@ -96,6 +98,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
setSearchQuery, setSearchQuery,
setContributors, setContributors,
setStatuses, setStatuses,
setSortBy,
clearFilters, clearFilters,
hasActiveFilters, hasActiveFilters,
} = usePRFiltering(prs, getReviewStateForPR); } = usePRFiltering(prs, getReviewStateForPR);
@@ -226,6 +229,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
onSearchChange={setSearchQuery} onSearchChange={setSearchQuery}
onContributorsChange={setContributors} onContributorsChange={setContributors}
onStatusesChange={setStatuses} onStatusesChange={setStatuses}
onSortChange={setSortBy}
onClearFilters={clearFilters} onClearFilters={clearFilters}
/> />
<PRList <PRList
@@ -236,6 +240,8 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
error={error} error={error}
getReviewStateForPR={getReviewStateForPR} getReviewStateForPR={getReviewStateForPR}
onSelectPR={selectPR} onSelectPR={selectPR}
onLoadMore={loadMore}
isLoadingMore={isLoadingMore}
/> />
</div> </div>
} }
@@ -4,7 +4,7 @@
* Multi-select dropdowns with visible chip selections * Multi-select dropdowns with visible chip selections
*/ */
import { useState, useMemo, useRef, useCallback } from 'react'; import { useState, useMemo, useRef, useCallback, useEffect } from 'react';
import { import {
Search, Search,
Users, Users,
@@ -17,7 +17,10 @@ import {
X, X,
Filter, Filter,
Check, Check,
Loader2 Loader2,
ArrowUpDown,
Clock,
FileCode
} from 'lucide-react'; } from 'lucide-react';
import { Input } from '../../ui/input'; import { Input } from '../../ui/input';
import { Badge } from '../../ui/badge'; import { Badge } from '../../ui/badge';
@@ -29,7 +32,7 @@ import {
DropdownMenuTrigger, DropdownMenuTrigger,
} from '../../ui/dropdown-menu'; } from '../../ui/dropdown-menu';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
import type { PRFilterState, PRStatusFilter } from '../hooks/usePRFiltering'; import type { PRFilterState, PRStatusFilter, PRSortOption } from '../hooks/usePRFiltering';
import { cn } from '../../../lib/utils'; import { cn } from '../../../lib/utils';
interface PRFilterBarProps { interface PRFilterBarProps {
@@ -39,6 +42,7 @@ interface PRFilterBarProps {
onSearchChange: (query: string) => void; onSearchChange: (query: string) => void;
onContributorsChange: (contributors: string[]) => void; onContributorsChange: (contributors: string[]) => void;
onStatusesChange: (statuses: PRStatusFilter[]) => void; onStatusesChange: (statuses: PRStatusFilter[]) => void;
onSortChange: (sortBy: PRSortOption) => void;
onClearFilters: () => void; onClearFilters: () => void;
} }
@@ -59,6 +63,17 @@ const STATUS_OPTIONS: Array<{
{ value: 'ready_for_followup', labelKey: 'prReview.readyForFollowup', icon: RefreshCw, color: 'text-cyan-400', bgColor: 'bg-cyan-500/20' }, { value: 'ready_for_followup', labelKey: 'prReview.readyForFollowup', icon: RefreshCw, color: 'text-cyan-400', bgColor: 'bg-cyan-500/20' },
]; ];
// Sort options
const SORT_OPTIONS: Array<{
value: PRSortOption;
labelKey: string;
icon: typeof Clock;
}> = [
{ value: 'newest', labelKey: 'prReview.sort.newest', icon: Clock },
{ value: 'oldest', labelKey: 'prReview.sort.oldest', icon: Clock },
{ value: 'largest', labelKey: 'prReview.sort.largest', icon: FileCode },
];
/** /**
* Modern Filter Dropdown Component * Modern Filter Dropdown Component
*/ */
@@ -138,6 +153,13 @@ function FilterDropdown<T extends string>({
} }
}, [filteredItems, focusedIndex, toggleItem]); }, [filteredItems, focusedIndex, toggleItem]);
// Scroll focused item into view for keyboard navigation
useEffect(() => {
if (focusedIndex >= 0 && itemRefs.current[focusedIndex]) {
itemRefs.current[focusedIndex]?.scrollIntoView({ block: 'nearest' });
}
}, [focusedIndex]);
return ( return (
<DropdownMenu open={isOpen} onOpenChange={(open) => { <DropdownMenu open={isOpen} onOpenChange={(open) => {
setIsOpen(open); setIsOpen(open);
@@ -279,6 +301,127 @@ function FilterDropdown<T extends string>({
); );
} }
/**
* Single-select Sort Dropdown Component
*/
function SortDropdown({
value,
onChange,
options,
title,
}: {
value: PRSortOption;
onChange: (value: PRSortOption) => void;
options: typeof SORT_OPTIONS;
title: string;
}) {
const { t } = useTranslation('common');
const [isOpen, setIsOpen] = useState(false);
const [focusedIndex, setFocusedIndex] = useState(-1);
const currentOption = options.find((opt) => opt.value === value) || options[0];
const handleKeyDown = useCallback((e: React.KeyboardEvent) => {
if (options.length === 0) return;
switch (e.key) {
case 'ArrowDown':
e.preventDefault();
setFocusedIndex((prev) => (prev < options.length - 1 ? prev + 1 : 0));
break;
case 'ArrowUp':
e.preventDefault();
setFocusedIndex((prev) => (prev > 0 ? prev - 1 : options.length - 1));
break;
case 'Enter':
case ' ':
e.preventDefault();
if (focusedIndex >= 0 && focusedIndex < options.length) {
onChange(options[focusedIndex].value);
setIsOpen(false);
}
break;
case 'Escape':
setIsOpen(false);
break;
}
}, [options, focusedIndex, onChange]);
return (
<DropdownMenu
open={isOpen}
onOpenChange={(open) => {
setIsOpen(open);
if (open) {
// Focus current selection on open for better keyboard UX
setFocusedIndex(options.findIndex((o) => o.value === value));
} else {
setFocusedIndex(-1);
}
}}
>
<DropdownMenuTrigger asChild>
<Button
variant="outline"
size="sm"
className="h-8 justify-start border-dashed bg-transparent"
>
<ArrowUpDown className="mr-2 h-4 w-4 text-muted-foreground" />
<span className="truncate">{title}</span>
<Separator orientation="vertical" className="mx-2 h-4" />
<Badge variant="secondary" className="rounded-sm px-1 font-normal">
{t(currentOption.labelKey)}
</Badge>
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="start" className="w-[180px] p-0">
<div className="px-3 py-2 border-b border-border/50">
<div className="text-xs font-semibold text-muted-foreground">
{title}
</div>
</div>
<div
className="p-1"
role="listbox"
tabIndex={0}
onKeyDown={handleKeyDown}
>
{options.map((option, index) => {
const isSelected = value === option.value;
const isFocused = focusedIndex === index;
const Icon = option.icon;
return (
<div
key={option.value}
role="option"
aria-selected={isSelected}
className={cn(
"relative flex cursor-pointer select-none items-center rounded-sm px-2 py-2 text-sm outline-none transition-colors hover:bg-accent hover:text-accent-foreground",
isSelected && "bg-accent/50",
isFocused && "bg-accent text-accent-foreground"
)}
onClick={() => {
onChange(option.value);
setIsOpen(false);
}}
>
<div className={cn(
"mr-2 flex h-4 w-4 items-center justify-center rounded-full border border-primary/30",
isSelected ? "bg-primary border-primary text-primary-foreground" : "opacity-50"
)}>
{isSelected && <Check className="h-2.5 w-2.5" />}
</div>
<Icon className="mr-2 h-3.5 w-3.5 text-muted-foreground" />
<span>{t(option.labelKey)}</span>
</div>
);
})}
</div>
</DropdownMenuContent>
</DropdownMenu>
);
}
export function PRFilterBar({ export function PRFilterBar({
filters, filters,
contributors, contributors,
@@ -286,6 +429,7 @@ export function PRFilterBar({
onSearchChange, onSearchChange,
onContributorsChange, onContributorsChange,
onStatusesChange, onStatusesChange,
onSortChange,
onClearFilters, onClearFilters,
}: PRFilterBarProps) { }: PRFilterBarProps) {
const { t } = useTranslation('common'); const { t } = useTranslation('common');
@@ -393,6 +537,16 @@ export function PRFilterBar({
/> />
</div> </div>
{/* Sort Dropdown */}
<div className="flex-shrink-0">
<SortDropdown
value={filters.sortBy}
onChange={onSortChange}
options={SORT_OPTIONS}
title={t('prReview.sort.label')}
/>
</div>
{/* Reset All */} {/* Reset All */}
{hasActiveFilters && ( {hasActiveFilters && (
<Button <Button
@@ -1,6 +1,7 @@
import { GitPullRequest, User, Clock, FileDiff } from 'lucide-react'; import { GitPullRequest, User, Clock, FileDiff, Loader2 } from 'lucide-react';
import { ScrollArea } from '../../ui/scroll-area'; import { ScrollArea } from '../../ui/scroll-area';
import { Badge } from '../../ui/badge'; import { Badge } from '../../ui/badge';
import { Button } from '../../ui/button';
import { cn } from '../../../lib/utils'; import { cn } from '../../../lib/utils';
import type { PRData, PRReviewProgress, PRReviewResult } from '../hooks/useGitHubPRs'; import type { PRData, PRReviewProgress, PRReviewResult } from '../hooks/useGitHubPRs';
import type { NewCommitsCheck } from '../../../../preload/api/modules/github-api'; import type { NewCommitsCheck } from '../../../../preload/api/modules/github-api';
@@ -170,6 +171,10 @@ interface PRListProps {
error: string | null; error: string | null;
getReviewStateForPR: (prNumber: number) => PRReviewInfo | null; getReviewStateForPR: (prNumber: number) => PRReviewInfo | null;
onSelectPR: (prNumber: number) => void; 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 { function formatDate(dateString: string): string {
@@ -200,6 +205,8 @@ export function PRList({
error, error,
getReviewStateForPR, getReviewStateForPR,
onSelectPR, onSelectPR,
onLoadMore,
isLoadingMore,
}: PRListProps) { }: PRListProps) {
const { t } = useTranslation('common'); const { t } = useTranslation('common');
@@ -306,12 +313,30 @@ export function PRList({
); );
})} })}
{/* Status indicator */} {/* Status indicator / Load More button */}
{prs.length > 0 && ( {prs.length > 0 && (
<div className="py-4 flex justify-center"> <div className="py-4 flex justify-center">
<span className="text-xs text-muted-foreground opacity-50"> {hasMore && onLoadMore ? (
{hasMore ? t('prReview.maxPRsShown') : t('prReview.allPRsLoaded')} <Button
</span> 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>
)} )}
</div> </div>
@@ -23,6 +23,7 @@ interface UseGitHubPRsOptions {
interface UseGitHubPRsResult { interface UseGitHubPRsResult {
prs: PRData[]; prs: PRData[];
isLoading: boolean; isLoading: boolean;
isLoadingMore: boolean; // Loading additional PRs via pagination
isLoadingPRDetails: boolean; // Loading full PR details including files isLoadingPRDetails: boolean; // Loading full PR details including files
error: string | null; error: string | null;
selectedPR: PRData | null; selectedPR: PRData | null;
@@ -38,6 +39,7 @@ interface UseGitHubPRsResult {
hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist
selectPR: (prNumber: number | null) => void; selectPR: (prNumber: number | null) => void;
refresh: () => Promise<void>; refresh: () => Promise<void>;
loadMore: () => Promise<void>; // Load next page of PRs
runReview: (prNumber: number) => void; runReview: (prNumber: number) => void;
runFollowupReview: (prNumber: number) => void; runFollowupReview: (prNumber: number) => void;
checkNewCommits: (prNumber: number) => Promise<NewCommitsCheck>; checkNewCommits: (prNumber: number) => Promise<NewCommitsCheck>;
@@ -76,6 +78,8 @@ export function useGitHubPRs(
const [isConnected, setIsConnected] = useState(false); const [isConnected, setIsConnected] = useState(false);
const [repoFullName, setRepoFullName] = useState<string | null>(null); const [repoFullName, setRepoFullName] = useState<string | null>(null);
const [hasMore, setHasMore] = useState(false); 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 // Track previous isActive state to detect tab navigation
const wasActiveRef = useRef(isActive); const wasActiveRef = useRef(isActive);
@@ -85,6 +89,10 @@ export function useGitHubPRs(
const currentFetchPRNumberRef = useRef<number | null>(null); const currentFetchPRNumberRef = useRef<number | null>(null);
// AbortController for cancelling pending checkNewCommits calls on rapid PR switching // AbortController for cancelling pending checkNewCommits calls on rapid PR switching
const checkNewCommitsAbortRef = useRef<AbortController | null>(null); const checkNewCommitsAbortRef = useRef<AbortController | null>(null);
// Track current projectId for staleness checks in async operations
const currentProjectIdRef = useRef(projectId);
// Counter to detect stale loadMore responses after a refresh
const fetchGenerationRef = useRef(0);
// Get PR review state from the global store // Get PR review state from the global store
const prReviews = usePRReviewStore((state) => state.prReviews); const prReviews = usePRReviewStore((state) => state.prReviews);
@@ -143,6 +151,9 @@ export function useGitHubPRs(
async () => { async () => {
if (!projectId) return; if (!projectId) return;
// Increment generation to invalidate any in-flight loadMore requests
fetchGenerationRef.current += 1;
setIsLoading(true); setIsLoading(true);
setError(null); setError(null);
@@ -159,6 +170,8 @@ export function useGitHubPRs(
if (result) { if (result) {
// Use hasNextPage from API to determine if more PRs exist // Use hasNextPage from API to determine if more PRs exist
setHasMore(result.hasNextPage); setHasMore(result.hasNextPage);
// Store endCursor for pagination
setEndCursor(result.endCursor ?? null);
setPrs(result.prs); setPrs(result.prs);
// Batch preload review results for PRs not in store (single IPC call) // Batch preload review results for PRs not in store (single IPC call)
@@ -223,11 +236,15 @@ export function useGitHubPRs(
// Reset state and selected PR when project changes // Reset state and selected PR when project changes
useEffect(() => { useEffect(() => {
currentProjectIdRef.current = projectId;
fetchGenerationRef.current += 1;
hasLoadedRef.current = false; hasLoadedRef.current = false;
setHasMore(false); setHasMore(false);
setEndCursor(null);
setPrs([]); setPrs([]);
setSelectedPRNumber(null); setSelectedPRNumber(null);
setSelectedPRDetails(null); setSelectedPRDetails(null);
setIsLoadingMore(false);
currentFetchPRNumberRef.current = null; currentFetchPRNumberRef.current = null;
// Cancel any pending checkNewCommits request // Cancel any pending checkNewCommits request
if (checkNewCommitsAbortRef.current) { if (checkNewCommitsAbortRef.current) {
@@ -377,6 +394,91 @@ export function useGitHubPRs(
await fetchPRs(); await fetchPRs();
}, [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( const runReview = useCallback(
(prNumber: number) => { (prNumber: number) => {
if (!projectId) return; if (!projectId) return;
@@ -567,6 +669,7 @@ export function useGitHubPRs(
return { return {
prs, prs,
isLoading, isLoading,
isLoadingMore,
isLoadingPRDetails, isLoadingPRDetails,
error, error,
selectedPR, selectedPR,
@@ -582,6 +685,7 @@ export function useGitHubPRs(
hasMore, hasMore,
selectPR, selectPR,
refresh, refresh,
loadMore,
runReview, runReview,
runFollowupReview, runFollowupReview,
checkNewCommits, checkNewCommits,
@@ -16,10 +16,13 @@ export type PRStatusFilter =
| 'ready_to_merge' | 'ready_to_merge'
| 'ready_for_followup'; | 'ready_for_followup';
export type PRSortOption = 'newest' | 'oldest' | 'largest';
export interface PRFilterState { export interface PRFilterState {
searchQuery: string; searchQuery: string;
contributors: string[]; contributors: string[];
statuses: PRStatusFilter[]; statuses: PRStatusFilter[];
sortBy: PRSortOption;
} }
interface PRReviewInfo { interface PRReviewInfo {
@@ -32,6 +35,7 @@ const DEFAULT_FILTERS: PRFilterState = {
searchQuery: '', searchQuery: '',
contributors: [], contributors: [],
statuses: [], statuses: [],
sortBy: 'newest',
}; };
/** /**
@@ -98,9 +102,9 @@ export function usePRFiltering(
); );
}, [prs]); }, [prs]);
// Filter PRs based on current filters // Filter and sort PRs based on current filters
const filteredPRs = useMemo(() => { const filteredPRs = useMemo(() => {
return prs.filter(pr => { const filtered = prs.filter(pr => {
// Search filter - matches title or body // Search filter - matches title or body
if (filters.searchQuery) { if (filters.searchQuery) {
const query = filters.searchQuery.toLowerCase(); const query = filters.searchQuery.toLowerCase();
@@ -142,6 +146,36 @@ export function usePRFiltering(
return true; return true;
}); });
// Pre-compute timestamps to avoid creating Date objects on every comparison
const timestamps = new Map(
filtered.map((pr) => [pr.number, new Date(pr.createdAt).getTime()])
);
// Sort the filtered results
return filtered.sort((a, b) => {
const aTime = timestamps.get(a.number)!;
const bTime = timestamps.get(b.number)!;
switch (filters.sortBy) {
case 'newest':
// Sort by createdAt descending (most recent first)
return bTime - aTime;
case 'oldest':
// Sort by createdAt ascending (oldest first)
return aTime - bTime;
case 'largest': {
// Sort by total changes (additions + deletions) descending
const aChanges = (a.additions || 0) + (a.deletions || 0);
const bChanges = (b.additions || 0) + (b.deletions || 0);
if (bChanges !== aChanges) return bChanges - aChanges;
// Secondary sort by createdAt (newest first) for stable ordering
return bTime - aTime;
}
default:
return 0;
}
});
}, [prs, filters, getReviewStateForPR]); }, [prs, filters, getReviewStateForPR]);
// Filter setters // Filter setters
@@ -157,8 +191,15 @@ export function usePRFiltering(
setFiltersState(prev => ({ ...prev, statuses })); setFiltersState(prev => ({ ...prev, statuses }));
}, []); }, []);
const setSortBy = useCallback((sortBy: PRSortOption) => {
setFiltersState(prev => ({ ...prev, sortBy }));
}, []);
const clearFilters = useCallback(() => { const clearFilters = useCallback(() => {
setFiltersState(DEFAULT_FILTERS); setFiltersState((prev) => ({
...DEFAULT_FILTERS,
sortBy: prev.sortBy, // Preserve sort preference when clearing filters
}));
}, []); }, []);
const hasActiveFilters = useMemo(() => { const hasActiveFilters = useMemo(() => {
@@ -176,6 +217,7 @@ export function usePRFiltering(
setSearchQuery, setSearchQuery,
setContributors, setContributors,
setStatuses, setStatuses,
setSortBy,
clearFilters, clearFilters,
hasActiveFilters, hasActiveFilters,
}; };
@@ -203,6 +203,7 @@ const browserMockAPI: ElectronAPI = {
onAutoFixComplete: () => () => {}, onAutoFixComplete: () => () => {},
onAutoFixError: () => () => {}, onAutoFixError: () => () => {},
listPRs: async () => ({ prs: [], hasNextPage: false }), listPRs: async () => ({ prs: [], hasNextPage: false }),
listMorePRs: async () => ({ prs: [], hasNextPage: false }),
getPR: async () => null, getPR: async () => null,
runPRReview: () => {}, runPRReview: () => {},
cancelPRReview: async () => true, cancelPRReview: async () => true,
@@ -388,6 +388,7 @@ export const IPC_CHANNELS = {
// GitHub PR Review operations // GitHub PR Review operations
GITHUB_PR_LIST: 'github:pr:list', GITHUB_PR_LIST: 'github:pr:list',
GITHUB_PR_LIST_MORE: 'github:pr:listMore', // Load more PRs (pagination)
GITHUB_PR_GET: 'github:pr:get', GITHUB_PR_GET: 'github:pr:get',
GITHUB_PR_GET_DIFF: 'github:pr:getDiff', GITHUB_PR_GET_DIFF: 'github:pr:getDiff',
GITHUB_PR_REVIEW: 'github:pr:review', GITHUB_PR_REVIEW: 'github:pr:review',
@@ -243,6 +243,12 @@
"selectedCount": "{{count}} selected", "selectedCount": "{{count}} selected",
"noResultsFound": "No results found", "noResultsFound": "No results found",
"reset": "Reset", "reset": "Reset",
"sort": {
"label": "Sort",
"newest": "Newest",
"oldest": "Oldest",
"largest": "Largest"
},
"pullRequests": "Pull Requests", "pullRequests": "Pull Requests",
"open": "open", "open": "open",
"selectPRToView": "Select a pull request to view details", "selectPRToView": "Select a pull request to view details",
@@ -361,6 +367,8 @@
"branchUpdateFailed": "Failed to update branch", "branchUpdateFailed": "Failed to update branch",
"allPRsLoaded": "All PRs loaded", "allPRsLoaded": "All PRs loaded",
"maxPRsShown": "Showing first 100 PRs", "maxPRsShown": "Showing first 100 PRs",
"loadMore": "Load More",
"loadingMore": "Loading...",
"workflowsAwaitingApproval": "{{count}} Workflow Awaiting Approval", "workflowsAwaitingApproval": "{{count}} Workflow Awaiting Approval",
"workflowsAwaitingApproval_plural": "{{count}} Workflows Awaiting Approval", "workflowsAwaitingApproval_plural": "{{count}} Workflows Awaiting Approval",
"blockedByWorkflows": "Blocked", "blockedByWorkflows": "Blocked",
@@ -243,6 +243,12 @@
"selectedCount": "{{count}} sélectionné(s)", "selectedCount": "{{count}} sélectionné(s)",
"noResultsFound": "Aucun résultat trouvé", "noResultsFound": "Aucun résultat trouvé",
"reset": "Réinitialiser", "reset": "Réinitialiser",
"sort": {
"label": "Trier",
"newest": "Plus récent",
"oldest": "Plus ancien",
"largest": "Plus grand"
},
"pullRequests": "Pull Requests", "pullRequests": "Pull Requests",
"open": "ouvert", "open": "ouvert",
"selectPRToView": "Sélectionnez une pull request pour voir les détails", "selectPRToView": "Sélectionnez une pull request pour voir les détails",
@@ -370,6 +376,8 @@
"branchUpdateFailed": "Échec de la mise à jour de la branche", "branchUpdateFailed": "Échec de la mise à jour de la branche",
"allPRsLoaded": "Tous les PRs chargés", "allPRsLoaded": "Tous les PRs chargés",
"maxPRsShown": "Affichage des 100 premières PRs", "maxPRsShown": "Affichage des 100 premières PRs",
"loadMore": "Charger plus",
"loadingMore": "Chargement...",
"workflowsAwaitingApproval": "{{count}} workflow en attente d'approbation", "workflowsAwaitingApproval": "{{count}} workflow en attente d'approbation",
"workflowsAwaitingApproval_plural": "{{count}} workflows en attente d'approbation", "workflowsAwaitingApproval_plural": "{{count}} workflows en attente d'approbation",
"blockedByWorkflows": "Bloqué", "blockedByWorkflows": "Bloqué",