From 7720ed136ce3160d2eb59b7c4476756ff4563bb7 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:30:22 -0700 Subject: [PATCH] feat(tasks): server-side scope filtering, pagination, and draft bug fix (#1135) Co-authored-by: Orca --- src/main/github/client.ts | 114 ++++++- src/main/ipc/github.ts | 10 +- src/preload/api-types.d.ts | 2 + src/preload/index.ts | 4 + src/renderer/src/components/TaskPage.tsx | 297 +++++++++++++++--- .../src/components/sidebar/SidebarNav.tsx | 4 +- src/renderer/src/hooks/useComposerState.ts | 3 +- src/renderer/src/lib/new-workspace.ts | 14 +- src/renderer/src/store/slices/github.ts | 75 ++++- src/renderer/src/store/slices/ui.ts | 18 +- src/shared/task-query.test.ts | 8 + src/shared/task-query.ts | 3 + src/shared/work-items.ts | 9 + 13 files changed, 485 insertions(+), 76 deletions(-) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index e77d30efa..2a3216ee1 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -216,8 +216,9 @@ function buildWorkItemListArgs(args: { ownerRepo: { owner: string; repo: string } | null limit: number query: ParsedTaskQuery + before?: string }): string[] { - const { kind, ownerRepo, limit, query } = args + const { kind, ownerRepo, limit, query, before } = args const fields = kind === 'issue' ? 'number,title,state,url,labels,updatedAt,author' @@ -249,20 +250,20 @@ function buildWorkItemListArgs(args: { out.push('--label', label) } } - if ( - kind === 'pr' && - query.scope === 'pr' && - query.state === 'open' && - query.freeText === '' && - !query.reviewRequested && - !query.reviewedBy - ) { + // Why: only add --draft when the user explicitly typed `is:draft`. Previously + // this fired for any PR-scoped open query, which made `is:pr is:open` (the + // "PRs" preset) silently filter to drafts-only. + if (kind === 'pr' && query.draft) { out.push('--draft') } - // review-requested and reviewed-by are not supported as standalone gh CLI flags, - // so they must be passed as GitHub search qualifiers via --search. const searchParts: string[] = [] + // Why: cursor-based pagination. GitHub search supports updated: { const fetchers: Promise[] = [] const issueScope = query.scope !== 'pr' @@ -371,7 +373,7 @@ async function listQueriedWorkItems( if (issueScope) { fetchers.push( (async () => { - const args = buildWorkItemListArgs({ kind: 'issue', ownerRepo, limit, query }) + const args = buildWorkItemListArgs({ kind: 'issue', ownerRepo, limit, query, before }) try { const { stdout } = await ghExecFileAsync(args, { cwd: repoPath }) return (JSON.parse(stdout) as Record[]).map(mapIssueWorkItem) @@ -385,7 +387,7 @@ async function listQueriedWorkItems( if (prScope) { fetchers.push( (async () => { - const args = buildWorkItemListArgs({ kind: 'pr', ownerRepo, limit, query }) + const args = buildWorkItemListArgs({ kind: 'pr', ownerRepo, limit, query, before }) try { const { stdout } = await ghExecFileAsync(args, { cwd: repoPath }) return (JSON.parse(stdout) as Record[]).map((item) => @@ -405,7 +407,8 @@ async function listQueriedWorkItems( export async function listWorkItems( repoPath: string, limit = 24, - query?: string + query?: string, + before?: string ): Promise { const ownerRepo = await getOwnerRepo(repoPath) const trimmedQuery = query?.trim() ?? '' @@ -420,7 +423,86 @@ export async function listWorkItems( } const parsedQuery = parseTaskQuery(trimmedQuery) - return await listQueriedWorkItems(repoPath, ownerRepo, parsedQuery, limit) + return await listQueriedWorkItems(repoPath, ownerRepo, parsedQuery, limit, before) + } finally { + release() + } +} + +function buildSearchQueryString( + ownerRepo: { owner: string; repo: string }, + query: ParsedTaskQuery +): string { + const parts: string[] = [`repo:${ownerRepo.owner}/${ownerRepo.repo}`] + if (query.scope === 'pr') { + parts.push('is:pull-request') + } else if (query.scope === 'issue') { + parts.push('is:issue') + } + if (query.state === 'open') { + parts.push('is:open') + } else if (query.state === 'closed') { + parts.push('is:closed') + } else if (query.state === 'merged') { + parts.push('is:merged') + } + if (query.draft) { + parts.push('draft:true') + } + if (query.assignee) { + parts.push(`assignee:${query.assignee}`) + } + if (query.author) { + parts.push(`author:${query.author}`) + } + if (query.reviewRequested) { + parts.push(`review-requested:${query.reviewRequested}`) + } + if (query.reviewedBy) { + parts.push(`reviewed-by:${query.reviewedBy}`) + } + for (const label of query.labels) { + parts.push(`label:${label}`) + } + if (query.freeText) { + parts.push(query.freeText) + } + return parts.join(' ') +} + +// Why: uses GitHub's search API to get total_count without fetching items. +// This powers the pagination bar so the user sees total pages upfront. +// Cached for 120s to avoid burning the search rate limit (30 req/min). +export async function countWorkItems(repoPath: string, query?: string): Promise { + const ownerRepo = await getOwnerRepo(repoPath) + if (!ownerRepo) { + return 0 + } + + const trimmedQuery = query?.trim() ?? '' + const parsedQuery = trimmedQuery ? parseTaskQuery(trimmedQuery) : null + + const searchQ = parsedQuery + ? buildSearchQueryString(ownerRepo, parsedQuery) + : `repo:${ownerRepo.owner}/${ownerRepo.repo} is:open` + + await acquire() + try { + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent(searchQ)}&per_page=1`, + '--jq', + '.total_count' + ], + { cwd: repoPath } + ) + return parseInt(stdout.trim(), 10) || 0 + } catch (err) { + console.warn('countWorkItems failed:', err) + return 0 } finally { release() } diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index 95a81906e..727987f02 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -9,6 +9,7 @@ import { getRepoSlug, listIssues, listWorkItems, + countWorkItems, getWorkItem, createIssue, updateIssue, @@ -76,12 +77,17 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi ipcMain.handle( 'gh:listWorkItems', - (_event, args: { repoPath: string; limit?: number; query?: string }) => { + (_event, args: { repoPath: string; limit?: number; query?: string; before?: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return listWorkItems(repo.path, args.limit, args.query) + return listWorkItems(repo.path, args.limit, args.query, args.before) } ) + ipcMain.handle('gh:countWorkItems', (_event, args: { repoPath: string; query?: string }) => { + const repo = assertRegisteredRepo(args.repoPath, store) + return countWorkItems(repo.path, args.query) + }) + ipcMain.handle('gh:workItem', (_event, args: { repoPath: string; number: number }) => { const repo = assertRegisteredRepo(args.repoPath, store) return getWorkItem(repo.path, args.number) diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index 4fa7e5582..0114a0468 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -399,10 +399,12 @@ export type PreloadApi = { title: string body: string }) => Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }> + countWorkItems: (args: { repoPath: string; query?: string }) => Promise listWorkItems: (args: { repoPath: string limit?: number query?: string + before?: string }) => Promise[]> prChecks: (args: { repoPath: string diff --git a/src/preload/index.ts b/src/preload/index.ts index be2557d5f..b9a34e6dc 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -391,10 +391,14 @@ const api = { }): Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }> => ipcRenderer.invoke('gh:createIssue', args), + countWorkItems: (args: { repoPath: string; query?: string }): Promise => + ipcRenderer.invoke('gh:countWorkItems', args), + listWorkItems: (args: { repoPath: string limit?: number query?: string + before?: string }): Promise => ipcRenderer.invoke('gh:listWorkItems', args), prChecks: (args: { diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 4fde45ffb..69bfec2b6 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -6,6 +6,8 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ArrowRight, ChevronDown, + ChevronLeft, + ChevronRight, CircleDot, EllipsisVertical, ExternalLink, @@ -54,7 +56,12 @@ import { stripRepoQualifiers } from '../../../shared/task-query' import GitHubItemDrawer from '@/components/GitHubItemDrawer' import LinearItemDrawer from '@/components/LinearItemDrawer' import { cn } from '@/lib/utils' -import { getLinkedWorkItemSuggestedName, getTaskPresetQuery } from '@/lib/new-workspace' +import { + getLinkedWorkItemSuggestedName, + getTaskPresetQuery, + PER_REPO_FETCH_LIMIT, + CROSS_REPO_DISPLAY_LIMIT +} from '@/lib/new-workspace' import type { LinkedWorkItemSummary } from '@/lib/new-workspace' import { launchWorkItemDirect } from '@/lib/launch-work-item-direct' import { isGitRepoKind } from '../../../shared/repo-kind' @@ -117,7 +124,7 @@ const LINEAR_PRESETS: LinearPreset[] = [ ] const TASK_SEARCH_DEBOUNCE_MS = 300 -const WORK_ITEM_LIMIT = 36 +const LINEAR_ITEM_LIMIT = 36 // Why: Intl.RelativeTimeFormat allocation is non-trivial, and previously we // built a new formatter per work-item row render. Hoisting to module scope @@ -493,6 +500,110 @@ function LinearPriorityCell({ issue }: { issue: LinearIssue }): React.JSX.Elemen ) } +// Why: builds the page number array with ellipsis gaps, matching GitHub's +// pagination pattern: always show first page, last page, and a window of +// pages around the current page with "..." gaps between distant ranges. +function getPageNumbers(current: number, total: number): (number | 'ellipsis')[] { + if (total <= 9) { + return Array.from({ length: total }, (_, i) => i) + } + const pages = new Set() + pages.add(0) + pages.add(total - 1) + for (let i = Math.max(0, current - 2); i <= Math.min(total - 1, current + 2); i++) { + pages.add(i) + } + const sorted = [...pages].sort((a, b) => a - b) + const result: (number | 'ellipsis')[] = [] + for (let i = 0; i < sorted.length; i++) { + if (i > 0 && sorted[i] - sorted[i - 1] > 1) { + result.push('ellipsis') + } + result.push(sorted[i]) + } + return result +} + +function PaginationBar({ + currentPage, + totalPages, + loadingTarget, + onPageChange +}: { + currentPage: number + totalPages: number + loadingTarget: number | null + onPageChange: (page: number) => void +}): React.JSX.Element { + const pageNumbers = getPageNumbers(currentPage, totalPages) + const btnClass = + 'inline-flex items-center gap-0.5 rounded-md px-2 py-1 text-sm text-muted-foreground transition hover:bg-muted/60 hover:text-foreground disabled:pointer-events-none disabled:opacity-40' + const numClass = (page: number): string => + cn( + 'inline-flex size-8 items-center justify-center rounded-md text-sm transition', + page === currentPage + ? 'bg-primary text-primary-foreground font-medium' + : 'text-muted-foreground hover:bg-muted/60 hover:text-foreground' + ) + + return ( + + ) +} + export default function TaskPage(): React.JSX.Element { const settings = useAppStore((s) => s.settings) const pageData = useAppStore((s) => s.taskPageData) @@ -639,26 +750,32 @@ export default function TaskPage(): React.JSX.Element { // user clicking the refresh button (force=true) vs. re-running for any // other reason — e.g. a repo change while the nonce happens to be > 0. const lastFetchedNonceRef = useRef(-1) - // Why: seed from the SWR cache across every initially-selected repo so the - // first paint shows the merged-and-sorted view instantly when all repos are - // already cached. Any missing cache entry simply contributes nothing here - // and will be filled in by the effect's fetch. - const [workItems, setWorkItems] = useState(() => { + // Why: pages holds all fetched pages of work items. Page 0 is seeded from + // cache for instant first paint; subsequent pages are loaded via date cursors. + const [pages, setPages] = useState(() => { const trimmed = initialTaskQuery.trim() const merged: GitHubWorkItem[] = [] for (const r of selectedRepos) { - const cached = getCachedWorkItems(r.path, WORK_ITEM_LIMIT, trimmed) + const cached = getCachedWorkItems(r.path, PER_REPO_FETCH_LIMIT, trimmed) if (cached) { merged.push(...cached) } } if (merged.length === 0) { - return [] + return [[]] } - return [...merged] + const page0 = [...merged] .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()) - .slice(0, WORK_ITEM_LIMIT) + .slice(0, CROSS_REPO_DISPLAY_LIMIT) + return [page0] }) + const [currentPage, setCurrentPage] = useState(0) + const [paginationLoading, setPaginationLoading] = useState(false) + const [loadingTargetPage, setLoadingTargetPage] = useState(null) + const [totalItemCount, setTotalItemCount] = useState(null) + const fetchWorkItemsNextPage = useAppStore((s) => s.fetchWorkItemsNextPage) + const countWorkItemsAcrossRepos = useAppStore((s) => s.countWorkItemsAcrossRepos) + // Why: clicking a GitHub row opens this drawer for a read-only preview. // Drawer's "Use" button routes through the same direct-launch flow as the // row-level "Use" CTA so behavior is consistent regardless of entry point. @@ -794,30 +911,103 @@ export default function TaskPage(): React.JSX.Element { ) const [linearConnectError, setLinearConnectError] = useState(null) - const filteredWorkItems = useMemo(() => { - if (!activeTaskPreset) { - return workItems - } + // Why: defense-in-depth safety net applied to the current page's items. + // The server-side query now includes is:issue / is:pr qualifiers so this + // filter is a no-op in the happy path. Kept as a guard against parser + // regressions or stale cache contamination. + const applyTypeFilter = useCallback( + (items: GitHubWorkItem[]) => { + if (!activeTaskPreset) { + return items + } + return items.filter((item) => { + if (activeTaskPreset === 'issues' || activeTaskPreset === 'my-issues') { + return item.type === 'issue' + } + if ( + activeTaskPreset === 'prs' || + activeTaskPreset === 'my-prs' || + activeTaskPreset === 'review' + ) { + return item.type === 'pr' + } + return true + }) + }, + [activeTaskPreset] + ) - return workItems.filter((item) => { - if (activeTaskPreset === 'issues') { - return item.type === 'issue' + const currentPageItems = useMemo(() => pages[currentPage] ?? [], [pages, currentPage]) + + const filteredWorkItems = useMemo( + () => applyTypeFilter(currentPageItems), + [applyTypeFilter, currentPageItems] + ) + + // Why: totalPages is derived from the search API count when available, + // so the pagination bar shows the full range (with ellipsis) upfront. + // Falls back to the loaded page count when the count hasn't returned yet. + const totalPages = + totalItemCount !== null + ? Math.max(pages.length, Math.ceil(totalItemCount / CROSS_REPO_DISPLAY_LIMIT)) + : pages.length + + // Why: loads the next page using the oldest item's updatedAt as a cursor. + // When targetPage is provided (from clicking a numbered page beyond loaded + // pages), it chains fetches until that page is loaded. + const handleLoadNextPage = useCallback( + async (targetPage?: number) => { + if (paginationLoading || selectedRepos.length === 0) { + return } - if (activeTaskPreset === 'review') { - return item.type === 'pr' + const lastPage = pages.at(-1) + if (!lastPage || lastPage.length === 0) { + return } - if (activeTaskPreset === 'my-issues') { - return item.type === 'issue' + const oldestItem = lastPage.at(-1) + if (!oldestItem?.updatedAt) { + return } - if (activeTaskPreset === 'prs') { - return item.type === 'pr' + const q = stripRepoQualifiers(appliedTaskSearch.trim()) + const repoArgs = selectedRepos.map((r) => ({ repoId: r.id, path: r.path })) + + const target = targetPage ?? pages.length + setPaginationLoading(true) + setLoadingTargetPage(target) + try { + let cursor = oldestItem.updatedAt + let loadedPages = pages.length + const newPages: GitHubWorkItem[][] = [] + + while (loadedPages <= target) { + const { items } = await fetchWorkItemsNextPage( + repoArgs, + PER_REPO_FETCH_LIMIT, + CROSS_REPO_DISPLAY_LIMIT, + q, + cursor + ) + if (items.length === 0) { + break + } + newPages.push(items) + cursor = items.at(-1)!.updatedAt + loadedPages += 1 + } + + if (newPages.length > 0) { + setPages((prev) => [...prev, ...newPages]) + setCurrentPage(target < loadedPages ? target : loadedPages - 1) + } + } catch (err) { + console.error('Failed to load next page:', err) + } finally { + setPaginationLoading(false) + setLoadingTargetPage(null) } - if (activeTaskPreset === 'my-prs') { - return item.type === 'pr' - } - return true - }) - }, [activeTaskPreset, workItems]) + }, + [paginationLoading, selectedRepos, pages, appliedTaskSearch, fetchWorkItemsNextPage] + ) useEffect(() => { const timeout = window.setTimeout(() => { @@ -847,7 +1037,7 @@ export default function TaskPage(): React.JSX.Element { const preMerged: GitHubWorkItem[] = [] let anyUncached = false for (const r of selectedRepos) { - const cached = getCachedWorkItems(r.path, WORK_ITEM_LIMIT, q) + const cached = getCachedWorkItems(r.path, PER_REPO_FETCH_LIMIT, q) if (cached === null) { anyUncached = true } else { @@ -857,13 +1047,15 @@ export default function TaskPage(): React.JSX.Element { // Why: always replace — if preMerged is empty (e.g. query just changed and // no repo has a cache entry for it), we clear the previous query's rows // rather than leaving them on screen under the spinner. - setWorkItems( + const page0 = preMerged.length > 0 ? [...preMerged] .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()) - .slice(0, WORK_ITEM_LIMIT) + .slice(0, CROSS_REPO_DISPLAY_LIMIT) : [] - ) + setPages([page0]) + setCurrentPage(0) + setTotalItemCount(null) setTasksError(null) setFailedCount(0) // reset so a prior failure banner doesn't linger setTasksLoading(anyUncached) @@ -873,14 +1065,15 @@ export default function TaskPage(): React.JSX.Element { lastFetchedNonceRef.current = taskRefreshNonce const repoArgs = selectedRepos.map((r) => ({ repoId: r.id, path: r.path })) - void fetchWorkItemsAcrossRepos(repoArgs, WORK_ITEM_LIMIT, q, { + void fetchWorkItemsAcrossRepos(repoArgs, PER_REPO_FETCH_LIMIT, CROSS_REPO_DISPLAY_LIMIT, q, { force: forceRefresh && taskRefreshNonce > 0 }) .then(({ items, failedCount: failed }) => { if (cancelled) { return } - setWorkItems(items) + setPages([items]) + setCurrentPage(0) setFailedCount(failed) setTasksLoading(false) }) @@ -895,6 +1088,18 @@ export default function TaskPage(): React.JSX.Element { setTasksLoading(false) }) + // Why: fire-and-forget count query in parallel with the items fetch. + // The search API is cached 120s server-side so this doesn't add + // meaningful latency or rate-limit pressure. + void countWorkItemsAcrossRepos( + selectedRepos.map((r) => ({ path: r.path })), + q + ).then((count) => { + if (!cancelled) { + setTotalItemCount(count) + } + }) + return () => { cancelled = true } @@ -1124,8 +1329,8 @@ export default function TaskPage(): React.JSX.Element { const trimmed = appliedLinearSearch.trim() const request = trimmed.length > 0 - ? searchLinearIssues(trimmed, WORK_ITEM_LIMIT) - : listLinearIssues(activeLinearPreset, WORK_ITEM_LIMIT) + ? searchLinearIssues(trimmed, LINEAR_ITEM_LIMIT) + : listLinearIssues(activeLinearPreset, LINEAR_ITEM_LIMIT) void request .then((issues) => { @@ -1792,6 +1997,22 @@ export default function TaskPage(): React.JSX.Element { ) })} + + {/* Pagination controls — GitHub-style with ellipsis */} + {filteredWorkItems.length > 0 && !tasksLoading && totalPages > 1 ? ( + { + if (page < pages.length) { + setCurrentPage(page) + } else { + void handleLoadNextPage(page) + } + }} + /> + ) : null} ) : !linearStatusChecked ? ( diff --git a/src/renderer/src/components/sidebar/SidebarNav.tsx b/src/renderer/src/components/sidebar/SidebarNav.tsx index e15f13497..bd2a46c6f 100644 --- a/src/renderer/src/components/sidebar/SidebarNav.tsx +++ b/src/renderer/src/components/sidebar/SidebarNav.tsx @@ -4,7 +4,7 @@ import { useAppStore } from '@/store' import { useRepoMap } from '@/store/selectors' import { cn } from '@/lib/utils' import { isGitRepoKind } from '../../../../shared/repo-kind' -import { getTaskPresetQuery } from '@/lib/new-workspace' +import { getTaskPresetQuery, PER_REPO_FETCH_LIMIT } from '@/lib/new-workspace' function LinearIcon({ className }: { className?: string }): React.JSX.Element { return ( @@ -42,7 +42,7 @@ const SidebarNav = React.memo(function SidebarNav() { prefetchWorkItems( firstGitRepo.id, firstGitRepo.path, - 36, + PER_REPO_FETCH_LIMIT, getTaskPresetQuery(defaultTaskViewPreset) ) } diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 4aeda90c8..86de65759 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -30,6 +30,7 @@ import { getLinkedWorkItemSuggestedName, getSetupConfig, getWorkspaceSeedName, + PER_REPO_FETCH_LIMIT, renderIssueCommandTemplate, type LinkedWorkItemSummary } from '@/lib/new-workspace' @@ -549,7 +550,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS if (!selectedRepo?.path || selectedRepo.connectionId) { return } - prefetchWorkItems(selectedRepo.id, selectedRepo.path, 36, 'is:pr is:open') + prefetchWorkItems(selectedRepo.id, selectedRepo.path, PER_REPO_FETCH_LIMIT, 'is:pr is:open') }, [prefetchWorkItems, selectedRepo?.connectionId, selectedRepo?.id, selectedRepo?.path]) // Per-repo: resolve repo slug for GH URL mismatch detection. diff --git a/src/renderer/src/lib/new-workspace.ts b/src/renderer/src/lib/new-workspace.ts index e9d99b9d8..619c30e03 100644 --- a/src/renderer/src/lib/new-workspace.ts +++ b/src/renderer/src/lib/new-workspace.ts @@ -9,14 +9,20 @@ import type { OrcaHooks, TaskViewPresetId } from '../../../shared/types' * mapping here so the prefetch warms exactly the cache key the page will look * up on mount. */ +export { PER_REPO_FETCH_LIMIT, CROSS_REPO_DISPLAY_LIMIT } from '../../../shared/work-items' + export function getTaskPresetQuery(presetId: TaskViewPresetId | null): string { switch (presetId) { + case 'issues': + return 'is:issue is:open' case 'my-issues': - return 'assignee:@me is:open' - case 'review': - return 'review-requested:@me is:open' + return 'assignee:@me is:issue is:open' + case 'prs': + return 'is:pr is:open' case 'my-prs': - return 'author:@me is:open' + return 'author:@me is:pr is:open' + case 'review': + return 'review-requested:@me is:pr is:open' default: return 'is:open' } diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 149431cd8..d7f429dc4 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -10,7 +10,7 @@ import type { Worktree, GitHubWorkItem } from '../../../../shared/types' -import { sortWorkItemsByUpdatedAt } from '../../../../shared/work-items' +import { sortWorkItemsByUpdatedAt, PER_REPO_FETCH_LIMIT } from '../../../../shared/work-items' import { syncPRChecksStatus } from './github-checks' export type CacheEntry = { @@ -185,10 +185,27 @@ export type GitHubSlice = { */ fetchWorkItemsAcrossRepos: ( repos: { repoId: string; path: string }[], - limit: number, + perRepoLimit: number, + displayLimit: number, query: string, options?: FetchOptions ) => Promise<{ items: GitHubWorkItem[]; failedCount: number }> + /** + * Fetch the next page of work items using a date cursor. Does not cache — + * pagination pages are ephemeral and managed by TaskPage state. + */ + fetchWorkItemsNextPage: ( + repos: { repoId: string; path: string }[], + perRepoLimit: number, + displayLimit: number, + query: string, + before: string + ) => Promise<{ items: GitHubWorkItem[]; failedCount: number }> + /** + * Count total work items across repos using GitHub's search API. + * Returns the sum of per-repo counts for the given query. + */ + countWorkItemsAcrossRepos: (repos: { path: string }[], query: string) => Promise /** * Fire-and-forget prefetch used by UI entry points (hover/focus of the * "new workspace" buttons) to warm the cache before the page mounts. @@ -268,19 +285,21 @@ export const createGitHubSlice: StateCreator = (s return request }, - fetchWorkItemsAcrossRepos: async (repos, limit, query, options) => { + fetchWorkItemsAcrossRepos: async (repos, perRepoLimit, displayLimit, query, options) => { const state = get() let failedCount = 0 const perRepoResults = await Promise.all( repos.map(async (r) => { try { - return await state.fetchWorkItems(r.repoId, r.path, limit, query, options) + return await state.fetchWorkItems(r.repoId, r.path, perRepoLimit, query, options) } catch (err) { // Why: fall back to any cache entry (stale or not) before declaring // this repo failed. Matches single-repo behavior of silently serving // stale data on error. A repo is only counted as failed when it has // nothing at all to contribute. - const key = workItemsCacheKey(r.path, limit, query) + // Why: must use perRepoLimit (not displayLimit) so the cache key + // matches what fetchWorkItems wrote. + const key = workItemsCacheKey(r.path, perRepoLimit, query) const cached = get().workItemsCache[key]?.data if (cached) { console.warn(`[workItems] ${r.repoId} failed, serving cached:`, err) @@ -292,11 +311,53 @@ export const createGitHubSlice: StateCreator = (s } }) ) - const merged = sortWorkItemsByUpdatedAt(perRepoResults.flat()).slice(0, limit) + const merged = sortWorkItemsByUpdatedAt(perRepoResults.flat()).slice(0, displayLimit) return { items: merged, failedCount } }, - prefetchWorkItems: (repoId, repoPath, limit = 36, query = '') => { + fetchWorkItemsNextPage: async (repos, perRepoLimit, displayLimit, query, before) => { + let failedCount = 0 + const perRepoResults = await Promise.all( + repos.map(async (r) => { + await acquireWorkItemSlot() + try { + const raw = (await window.api.gh.listWorkItems({ + repoPath: r.path, + limit: perRepoLimit, + query: query || undefined, + before + })) as Omit[] + return raw.map((item): GitHubWorkItem => ({ ...item, repoId: r.repoId })) + } catch (err) { + console.warn(`[workItems] next page ${r.repoId} failed:`, err) + failedCount += 1 + return [] as GitHubWorkItem[] + } finally { + releaseWorkItemSlot() + } + }) + ) + const merged = sortWorkItemsByUpdatedAt(perRepoResults.flat()).slice(0, displayLimit) + return { items: merged, failedCount } + }, + + countWorkItemsAcrossRepos: async (repos, query) => { + const counts = await Promise.all( + repos.map(async (r) => { + try { + return await window.api.gh.countWorkItems({ + repoPath: r.path, + query: query || undefined + }) + } catch { + return 0 + } + }) + ) + return counts.reduce((sum, c) => sum + c, 0) + }, + + prefetchWorkItems: (repoId, repoPath, limit = PER_REPO_FETCH_LIMIT, query = '') => { const key = workItemsCacheKey(repoPath, limit, query) const cached = get().workItemsCache[key] // Skip when the cache is fresh or a request is already in flight. diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 6f5761629..5a31f1ba8 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -10,18 +10,24 @@ import type { UpdateStatus, WorktreeCardProperty } from '../../../../shared/types' +import { PER_REPO_FETCH_LIMIT } from '../../../../shared/work-items' -// Why: mirrors the preset→query mapping used by TaskPage's preset buttons. +// Why: mirrors the preset→query mapping in getTaskPresetQuery (new-workspace.ts). // Keeping a local copy here avoids a store ↔ lib circular import while letting // openTaskPage warm exactly the cache key the page will read on mount. +// Must stay in sync with getTaskPresetQuery — see DESIGN-gh-issues-improve.md. function presetToQuery(presetId: TaskViewPresetId | null): string { switch (presetId) { + case 'issues': + return 'is:issue is:open' case 'my-issues': - return 'assignee:@me is:open' - case 'review': - return 'review-requested:@me is:open' + return 'assignee:@me is:issue is:open' + case 'prs': + return 'is:pr is:open' case 'my-prs': - return 'author:@me is:open' + return 'author:@me is:pr is:open' + case 'review': + return 'review-requested:@me is:pr is:open' default: return 'is:open' } @@ -198,7 +204,7 @@ export const createUISlice: StateCreator = (set, get) const repo = targetRepoId ? state.repos.find((r) => r.id === targetRepoId) : null if (repo?.path) { const preset = state.settings?.defaultTaskViewPreset ?? 'all' - state.prefetchWorkItems(repo.id, repo.path, 36, presetToQuery(preset)) + state.prefetchWorkItems(repo.id, repo.path, PER_REPO_FETCH_LIMIT, presetToQuery(preset)) } }, closeTaskPage: () => diff --git a/src/shared/task-query.test.ts b/src/shared/task-query.test.ts index 5956ff81b..55d496ee5 100644 --- a/src/shared/task-query.test.ts +++ b/src/shared/task-query.test.ts @@ -47,6 +47,14 @@ describe('parseTaskQuery', () => { const parsed = parseTaskQuery('is:draft') expect(parsed.scope).toBe('pr') expect(parsed.state).toBe('open') + expect(parsed.draft).toBe(true) + }) + + it('is:pr is:open does not set draft', () => { + const parsed = parseTaskQuery('is:pr is:open') + expect(parsed.scope).toBe('pr') + expect(parsed.state).toBe('open') + expect(parsed.draft).toBe(false) }) it('extracts assignee, author, label, and review qualifiers', () => { diff --git a/src/shared/task-query.ts b/src/shared/task-query.ts index 968b86d56..ea0337de6 100644 --- a/src/shared/task-query.ts +++ b/src/shared/task-query.ts @@ -1,6 +1,7 @@ export type ParsedTaskQuery = { scope: 'all' | 'issue' | 'pr' state: 'open' | 'closed' | 'all' | 'merged' | null + draft: boolean assignee: string | null author: string | null reviewRequested: string | null @@ -23,6 +24,7 @@ export function parseTaskQuery(rawQuery: string): ParsedTaskQuery { const query: ParsedTaskQuery = { scope: 'all', state: null, + draft: false, assignee: null, author: null, reviewRequested: null, @@ -60,6 +62,7 @@ export function parseTaskQuery(rawQuery: string): ParsedTaskQuery { if (normalized === 'is:draft') { query.scope = 'pr' query.state = 'open' + query.draft = true continue } diff --git a/src/shared/work-items.ts b/src/shared/work-items.ts index aebcb6058..f92b08982 100644 --- a/src/shared/work-items.ts +++ b/src/shared/work-items.ts @@ -1,3 +1,12 @@ +// Why: per-repo fetch budget for gh CLI calls. Kept in shared/ so the renderer's +// prefetch sites (SidebarNav, ui.ts openTaskPage) and the TaskPage all use the +// same value for cache-key alignment. +export const PER_REPO_FETCH_LIMIT = 36 + +// Why: how many items to show after cross-repo merge. Decoupled from the per-repo +// fetch limit so changing the display cap doesn't invalidate cache keys. +export const CROSS_REPO_DISPLAY_LIMIT = 100 + // Why: generic over the item shape because main-process callers emit items // without repoId (stamped by the renderer after IPC), while renderer callers // carry the full GitHubWorkItem. Both share only the updatedAt field needed