From 4c26ef626c0b298c6996f386f4c456864bf76514 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 7 Aug 2026 19:18:29 -0700 Subject: [PATCH] Extract GitHub task search commit debouncing into a hook (#13112) * Extract GitHub task search commit debouncing into a hook Move debounce logic from TaskPage into useGitHubTaskSearchCommit to prevent excessive GitHub API calls on every keystroke. Uses a 750ms idle window before committing search values. Add tests for the new hook. * Keep task rows visible while typing search query Removed premature row-hiding logic from the search input handler that was triggering before debounced queries fire. The handler now only updates the input state; debouncing and query timing are handled by a dedicated hook. Added e2e test verifying search idles before fetching and Enter doesn't double-fetch. * Test that GitHub task search commits cancel on disable and unmount Verify the useGitHubTaskSearchCommit hook properly cleans up pending commits when disabled or when the component unmounts. This prevents unnecessary GitHub API calls during normal user interaction. Also fix e2e test instrumentation to find the active repo through the worktree relationship rather than assuming the first repo with a path. --- src/renderer/src/components/TaskPage.tsx | 37 ++--- .../use-github-task-search-commit.test.ts | 59 +++++++ .../use-github-task-search-commit.ts | 30 ++++ tests/e2e/tasks-page.spec.ts | 146 ++++++++++++++++++ 4 files changed, 252 insertions(+), 20 deletions(-) create mode 100644 src/renderer/src/components/use-github-task-search-commit.test.ts create mode 100644 src/renderer/src/components/use-github-task-search-commit.ts diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 2c3917ab1..2b6aef96d 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -431,6 +431,7 @@ import { type LinearOrderBy, type LinearViewMode } from '@/components/task-page-localized-options' +import { useGitHubTaskSearchCommit } from '@/components/use-github-task-search-commit' function isGitLabMRFilter(value: GitLabTaskFilter | GitLabIssueFilter): value is GitLabTaskFilter { return value === 'opened' || value === 'merged' || value === 'closed' || value === 'all' @@ -6533,19 +6534,21 @@ export default function TaskPage(): React.JSX.Element { ] ) - useEffect(() => { - if (!taskResumeApplied) { - return - } - const timeout = window.setTimeout(() => { - const scoped = scopeGitHubTaskSearch(taskSearchInput, activeGithubTaskKind) + const commitTaskSearch = useCallback( + (value: string): void => { + const scoped = scopeGitHubTaskSearch(value, activeGithubTaskKind) if (scoped !== appliedTaskSearch) { setTasksFiltering(true) } setAppliedTaskSearch(scoped) - }, TASK_SEARCH_DEBOUNCE_MS) - return () => window.clearTimeout(timeout) - }, [activeGithubTaskKind, appliedTaskSearch, taskSearchInput, taskResumeApplied]) + }, + [activeGithubTaskKind, appliedTaskSearch] + ) + useGitHubTaskSearchCommit({ + enabled: taskResumeApplied, + onCommit: commitTaskSearch, + value: taskSearchInput + }) useEffect(() => { if (!taskResumeApplied) { @@ -7140,17 +7143,11 @@ export default function TaskPage(): React.JSX.Element { setTaskRefreshNonce((current) => current + 1) }, [activeGithubTaskKind, setTaskResumeState, taskSearchInput]) - const handleTaskSearchChange = useCallback( - (event: React.ChangeEvent): void => { - const next = event.target.value - const scoped = scopeGitHubTaskSearch(next, activeGithubTaskKind) - setTaskSearchInput(next) - setActiveTaskPreset(null) - // Why: visible rows are keyed by appliedTaskSearch, not the draft input; hide stale rows once the draft changes the query. - setTasksFiltering(scoped !== appliedTaskSearch) - }, - [activeGithubTaskKind, appliedTaskSearch] - ) + const handleTaskSearchChange = useCallback((event: React.ChangeEvent): void => { + const next = event.target.value + setTaskSearchInput(next) + setActiveTaskPreset(null) + }, []) const handleSetDefaultTaskPreset = useCallback( (presetId: TaskViewPresetId): void => { diff --git a/src/renderer/src/components/use-github-task-search-commit.test.ts b/src/renderer/src/components/use-github-task-search-commit.test.ts new file mode 100644 index 000000000..6885ca35f --- /dev/null +++ b/src/renderer/src/components/use-github-task-search-commit.test.ts @@ -0,0 +1,59 @@ +// @vitest-environment happy-dom + +import { act, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + GITHUB_TASK_SEARCH_IDLE_MS, + useGitHubTaskSearchCommit +} from './use-github-task-search-commit' + +describe('useGitHubTaskSearchCommit', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('commits only the final value after a full idle window', () => { + const onCommit = vi.fn() + const view = renderHook( + ({ value }) => useGitHubTaskSearchCommit({ enabled: true, onCommit, value }), + { initialProps: { value: 'r' } } + ) + + act(() => vi.advanceTimersByTime(400)) + view.rerender({ value: 'ra' }) + act(() => vi.advanceTimersByTime(400)) + view.rerender({ value: 'rate' }) + act(() => vi.advanceTimersByTime(GITHUB_TASK_SEARCH_IDLE_MS - 1)) + + expect(onCommit).not.toHaveBeenCalled() + + act(() => vi.advanceTimersByTime(1)) + expect(onCommit).toHaveBeenCalledOnce() + expect(onCommit).toHaveBeenCalledWith('rate') + }) + + it('cancels a pending commit when disabled', () => { + const onCommit = vi.fn() + const view = renderHook( + ({ enabled }) => useGitHubTaskSearchCommit({ enabled, onCommit, value: 'rate' }), + { initialProps: { enabled: true } } + ) + + act(() => vi.advanceTimersByTime(400)) + view.rerender({ enabled: false }) + + act(() => vi.runAllTimers()) + expect(onCommit).not.toHaveBeenCalled() + }) + + it('cancels a pending commit on unmount', () => { + const onCommit = vi.fn() + const view = renderHook(() => + useGitHubTaskSearchCommit({ enabled: true, onCommit, value: 'rate' }) + ) + + view.unmount() + act(() => vi.runAllTimers()) + + expect(onCommit).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/use-github-task-search-commit.ts b/src/renderer/src/components/use-github-task-search-commit.ts new file mode 100644 index 000000000..b6122a482 --- /dev/null +++ b/src/renderer/src/components/use-github-task-search-commit.ts @@ -0,0 +1,30 @@ +import { useEffect, useRef } from 'react' + +// Remote GitHub fan-out needs a longer idle window; Enter still commits immediately. +export const GITHUB_TASK_SEARCH_IDLE_MS = 750 + +type GitHubTaskSearchCommitOptions = { + enabled: boolean + onCommit: (value: string) => void + value: string +} + +export function useGitHubTaskSearchCommit({ + enabled, + onCommit, + value +}: GitHubTaskSearchCommitOptions): void { + const onCommitRef = useRef(onCommit) + // Keep latest callback without restarting the idle timer when identity changes. + useEffect(() => { + onCommitRef.current = onCommit + }, [onCommit]) + + useEffect(() => { + if (!enabled) { + return + } + const timeout = window.setTimeout(() => onCommitRef.current(value), GITHUB_TASK_SEARCH_IDLE_MS) + return () => window.clearTimeout(timeout) + }, [enabled, value]) +} diff --git a/tests/e2e/tasks-page.spec.ts b/tests/e2e/tasks-page.spec.ts index 9070d5341..7991cbf27 100644 --- a/tests/e2e/tasks-page.spec.ts +++ b/tests/e2e/tasks-page.spec.ts @@ -13,6 +13,11 @@ type RenderedTaskSource = { active: boolean } +type TaskSearchRequestProbe = { + countQueries: string[] + fetchQueries: string[] +} + const TASK_SOURCE_BY_LABEL: Record = { GitHub: 'github', GitLab: 'gitlab', @@ -49,6 +54,105 @@ async function getRenderedTaskSources( }, TASK_SOURCE_BY_LABEL) } +async function openInstrumentedGitHubTasksPage( + page: Parameters[0] +): Promise { + await page.evaluate(() => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const activeWorktree = Object.values(state.worktreesByRepo) + .flat() + .find((worktree) => worktree.id === state.activeWorktreeId) + const repo = state.repos.find((candidate) => candidate.id === activeWorktree?.repoId) + if (!repo || !state.settings) { + throw new Error('GitHub Tasks probe requires a ready repository and settings') + } + const probe: TaskSearchRequestProbe = { countQueries: [], fetchQueries: [] } + ;( + window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe } + ).__taskSearchRequestProbe = probe + const existingIssue = { + id: 'issue:999', + type: 'issue' as const, + number: 999, + title: 'Existing GitHub issue', + state: 'open' as const, + url: 'https://github.com/orca/e2e/issues/999', + labels: [], + updatedAt: '2026-08-08T00:00:00Z', + author: 'orca-e2e', + repoId: repo.id, + assignees: [], + reviewRequests: [] + } + + store.setState({ + repos: state.repos.map((candidate) => + candidate.id === repo.id + ? { ...candidate, upstream: { owner: 'orca', repo: 'e2e' } } + : candidate + ), + settings: { + ...state.settings, + defaultTaskSource: 'github', + defaultTaskViewPreset: 'issues', + visibleTaskProviders: ['github'] + }, + taskResumeState: { + ...state.taskResumeState, + githubItemsPreset: 'issues', + githubItemsQuery: 'is:issue is:open', + githubMode: 'items' + }, + prefetchWorkItems: () => undefined, + fetchWorkItemsAcrossRepos: async (_repos, _perRepoLimit, _displayLimit, query) => { + probe.fetchQueries.push(query) + return { items: [existingIssue], failedCount: 0, githubUnavailable: false } + }, + countWorkItemsAcrossRepos: async (_repos, query) => { + probe.countQueries.push(query) + return { totalCount: 1, totalPages: 1 } + } + }) + store + .getState() + .openTaskPage( + { taskSource: 'github', preselectedRepoId: repo.id }, + { recordTasksInteraction: false } + ) + }) +} + +async function readTaskSearchRequestProbe( + page: Parameters[0] +): Promise { + return page.evaluate(() => { + const probe = (window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe }) + .__taskSearchRequestProbe + if (!probe) { + throw new Error('Task search request probe is not installed') + } + return { countQueries: [...probe.countQueries], fetchQueries: [...probe.fetchQueries] } + }) +} + +async function resetTaskSearchRequestProbe( + page: Parameters[0] +): Promise { + await page.evaluate(() => { + const probe = (window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe }) + .__taskSearchRequestProbe + if (!probe) { + throw new Error('Task search request probe is not installed') + } + probe.countQueries.length = 0 + probe.fetchQueries.length = 0 + }) +} + test.describe('Tasks page', () => { test.beforeEach(async ({ orcaPage }) => { await waitForSessionReady(orcaPage) @@ -129,4 +233,46 @@ test.describe('Tasks page', () => { await expect(orcaPage.locator('.xterm').first()).toBeVisible({ timeout: 5_000 }) } }) + + test('GitHub search waits for idle, keeps rows visible, and Enter does not double-fetch', async ({ + orcaPage + }) => { + await openInstrumentedGitHubTasksPage(orcaPage) + + const input = orcaPage.getByPlaceholder('Search GitHub issues...') + const existingIssue = orcaPage.getByText('Existing GitHub issue', { exact: true }) + await expect(input).toBeVisible() + await expect(existingIssue).toBeVisible() + + await input.fill('') + await orcaPage.waitForTimeout(800) + await resetTaskSearchRequestProbe(orcaPage) + + await input.pressSequentially('rate', { delay: 400 }) + + expect(await readTaskSearchRequestProbe(orcaPage)).toEqual({ + countQueries: [], + fetchQueries: [] + }) + await expect(existingIssue).toBeVisible() + + await expect + .poll(async () => readTaskSearchRequestProbe(orcaPage), { timeout: 2_000 }) + .toEqual({ countQueries: ['is:issue rate'], fetchQueries: ['is:issue rate'] }) + + await resetTaskSearchRequestProbe(orcaPage) + await input.pressSequentially('x') + await input.press('Enter') + + await expect + .poll(async () => readTaskSearchRequestProbe(orcaPage), { timeout: 2_000 }) + .toEqual({ countQueries: ['is:issue ratex'], fetchQueries: ['is:issue ratex'] }) + await orcaPage.waitForTimeout(800) + expect(await readTaskSearchRequestProbe(orcaPage)).toEqual({ + countQueries: ['is:issue ratex'], + fetchQueries: ['is:issue ratex'] + }) + await expect(input).toHaveValue('is:issue ratex') + await expect(existingIssue).toBeVisible() + }) })