From fdddb3911565b3e6968d2580e7fe938b59a9c19f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 31 May 2026 10:32:43 -0700 Subject: [PATCH] perf: reset external automation runs during render (#4273) --- .../ExternalAutomationRunTable.tsx | 59 +++++----- ...xternal-automation-run-table-state.test.ts | 108 ++++++++++++++++++ .../external-automation-run-table-state.ts | 75 ++++++++++++ 3 files changed, 215 insertions(+), 27 deletions(-) create mode 100644 src/renderer/src/components/automations/external-automation-run-table-state.test.ts create mode 100644 src/renderer/src/components/automations/external-automation-run-table-state.ts diff --git a/src/renderer/src/components/automations/ExternalAutomationRunTable.tsx b/src/renderer/src/components/automations/ExternalAutomationRunTable.tsx index b3ce5133bf4..e9ea47d4014 100644 --- a/src/renderer/src/components/automations/ExternalAutomationRunTable.tsx +++ b/src/renderer/src/components/automations/ExternalAutomationRunTable.tsx @@ -10,6 +10,12 @@ import type { ExternalAutomationRun } from '../../../../shared/automations-types' import { formatAutomationDateTimeWithRelative } from './automation-page-parts' +import { + createExternalAutomationRunTableState, + resolveExternalAutomationFetchedRuns, + resolveExternalAutomationRunTableState, + updateExternalAutomationRunTablePage +} from './external-automation-run-table-state' const PAGE_SIZE = 8 @@ -88,25 +94,21 @@ export function ExternalAutomationRunTable({ onFetchRuns, onOpenRun }: ExternalAutomationRunTableProps): React.JSX.Element { - const [page, setPage] = useState(0) - const [selectedRunId, setSelectedRunId] = useState(job.runs[0]?.id ?? null) - const [fetchedRuns, setFetchedRuns] = useState(null) - const [fetchedTotalCount, setFetchedTotalCount] = useState(null) + const [tableState, setTableState] = useState(() => createExternalAutomationRunTableState(job)) const [isLoading, setIsLoading] = useState(false) - const [fetchError, setFetchError] = useState(null) const managerRef = useRef(manager) const jobRef = useRef(job) managerRef.current = manager jobRef.current = job - useEffect(() => { - setPage(0) - setSelectedRunId(job.runs[0]?.id ?? null) - setFetchedRuns(null) - setFetchedTotalCount(null) - setFetchError(null) - }, [job.id, job.runs]) + const resolvedTableState = resolveExternalAutomationRunTableState(tableState, job) + if (resolvedTableState !== tableState) { + // Why: manager rows can switch jobs while the table stays mounted; reset + // before paint so stale fetched rows/selection never flash for the new job. + setTableState(resolvedTableState) + } + const { page, selectedRunId, fetchedRuns, fetchedTotalCount, fetchError } = resolvedTableState useEffect(() => { if (!onFetchRuns) { @@ -114,7 +116,10 @@ export function ExternalAutomationRunTable({ } let cancelled = false setIsLoading(true) - setFetchError(null) + setTableState((current) => ({ + ...resolveExternalAutomationRunTableState(current, jobRef.current), + fetchError: null + })) void onFetchRuns({ manager: managerRef.current, job: jobRef.current, @@ -126,20 +131,18 @@ export function ExternalAutomationRunTable({ return } const nextPage = normalizeRunPage(result) - setFetchedRuns(nextPage.runs) - setFetchedTotalCount(nextPage.totalCount ?? null) - setSelectedRunId((current) => { - if (current && nextPage.runs.some((run) => run.id === current)) { - return current - } - return nextPage.runs[0]?.id ?? null - }) + setTableState((current) => + resolveExternalAutomationFetchedRuns(current, jobRef.current, nextPage) + ) }) .catch((error) => { if (!cancelled) { - setFetchedRuns(null) - setFetchedTotalCount(null) - setFetchError(error instanceof Error ? error.message : 'Failed to load runs.') + setTableState((current) => ({ + ...resolveExternalAutomationRunTableState(current, jobRef.current), + fetchedRuns: null, + fetchedTotalCount: null, + fetchError: error instanceof Error ? error.message : 'Failed to load runs.' + })) } }) .finally(() => { @@ -171,8 +174,7 @@ export function ExternalAutomationRunTable({ const pageEnd = Math.min(totalCount, page * PAGE_SIZE + visibleRuns.length) const handlePageChange = (nextPage: number): void => { - setPage(nextPage) - setSelectedRunId(null) + setTableState((current) => updateExternalAutomationRunTablePage(current, job, nextPage)) } return ( @@ -212,7 +214,10 @@ export function ExternalAutomationRunTable({ type="button" data-current={selectedRun?.id === run.id} onClick={() => { - setSelectedRunId(run.id) + setTableState((current) => ({ + ...resolveExternalAutomationRunTableState(current, job), + selectedRunId: run.id + })) onOpenRun?.(run) }} className={cn( diff --git a/src/renderer/src/components/automations/external-automation-run-table-state.test.ts b/src/renderer/src/components/automations/external-automation-run-table-state.test.ts new file mode 100644 index 00000000000..10da77d348f --- /dev/null +++ b/src/renderer/src/components/automations/external-automation-run-table-state.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from 'vitest' +import type { ExternalAutomationRun } from '../../../../shared/automations-types' +import { + createExternalAutomationRunTableState, + resolveExternalAutomationFetchedRuns, + resolveExternalAutomationRunTableState, + updateExternalAutomationRunTablePage +} from './external-automation-run-table-state' + +function run(id: string): ExternalAutomationRun { + return { + id, + managerId: 'manager-1', + provider: 'hermes', + jobId: 'job-1', + runAt: '2026-05-31T10:00:00.000Z', + status: 'completed', + outputPreview: `run ${id}`, + outputContent: null, + error: null, + outputPath: null + } +} + +function job( + id: string, + runs: readonly ExternalAutomationRun[] +): { + id: string + runs: readonly ExternalAutomationRun[] +} { + return { + id, + runs + } +} + +describe('external automation run table state', () => { + it('resets page, selection, fetched rows, and error when the job changes', () => { + const current = { + ...createExternalAutomationRunTableState(job('job-1', [run('run-1')])), + page: 2, + selectedRunId: 'run-9', + fetchedRuns: [run('run-9')], + fetchedTotalCount: 20, + fetchError: 'Failed' + } + + expect(resolveExternalAutomationRunTableState(current, job('job-2', [run('run-2')]))).toEqual({ + sourceJobId: 'job-2', + sourceRuns: [run('run-2')], + page: 0, + selectedRunId: 'run-2', + fetchedRuns: null, + fetchedTotalCount: null, + fetchError: null + }) + }) + + it('resets when the same job receives a new fallback run list', () => { + const firstRuns = [run('run-1')] + const nextRuns = [run('run-2')] + const current = { + ...createExternalAutomationRunTableState(job('job-1', firstRuns)), + selectedRunId: 'run-1' + } + + expect(resolveExternalAutomationRunTableState(current, job('job-1', nextRuns))).toMatchObject({ + sourceJobId: 'job-1', + sourceRuns: nextRuns, + page: 0, + selectedRunId: 'run-2', + fetchedRuns: null + }) + }) + + it('preserves selected fetched runs when loading another page response for the same job', () => { + const current = { + ...createExternalAutomationRunTableState(job('job-1', [run('fallback')])), + selectedRunId: 'run-2' + } + + expect( + resolveExternalAutomationFetchedRuns(current, job('job-1', current.sourceRuns), { + runs: [run('run-1'), run('run-2')], + totalCount: 12 + }) + ).toMatchObject({ + selectedRunId: 'run-2', + fetchedRuns: [run('run-1'), run('run-2')], + fetchedTotalCount: 12 + }) + }) + + it('clears selection when paging manually', () => { + const current = { + ...createExternalAutomationRunTableState(job('job-1', [run('run-1')])), + selectedRunId: 'run-1' + } + + expect( + updateExternalAutomationRunTablePage(current, job('job-1', current.sourceRuns), 3) + ).toMatchObject({ + page: 3, + selectedRunId: null + }) + }) +}) diff --git a/src/renderer/src/components/automations/external-automation-run-table-state.ts b/src/renderer/src/components/automations/external-automation-run-table-state.ts new file mode 100644 index 00000000000..ef25c085049 --- /dev/null +++ b/src/renderer/src/components/automations/external-automation-run-table-state.ts @@ -0,0 +1,75 @@ +import type { ExternalAutomationRun } from '../../../../shared/automations-types' + +type ExternalAutomationRunTableJob = { + id: string + runs: readonly ExternalAutomationRun[] +} + +type ExternalAutomationRunPageResult = { + runs: ExternalAutomationRun[] + totalCount?: number +} + +export type ExternalAutomationRunTableState = { + sourceJobId: string + sourceRuns: readonly ExternalAutomationRun[] + page: number + selectedRunId: string | null + fetchedRuns: ExternalAutomationRun[] | null + fetchedTotalCount: number | null + fetchError: string | null +} + +export function createExternalAutomationRunTableState( + job: ExternalAutomationRunTableJob +): ExternalAutomationRunTableState { + return { + sourceJobId: job.id, + sourceRuns: job.runs, + page: 0, + selectedRunId: job.runs[0]?.id ?? null, + fetchedRuns: null, + fetchedTotalCount: null, + fetchError: null + } +} + +export function resolveExternalAutomationRunTableState( + state: ExternalAutomationRunTableState, + job: ExternalAutomationRunTableJob +): ExternalAutomationRunTableState { + return state.sourceJobId === job.id && state.sourceRuns === job.runs + ? state + : createExternalAutomationRunTableState(job) +} + +export function updateExternalAutomationRunTablePage( + state: ExternalAutomationRunTableState, + job: ExternalAutomationRunTableJob, + page: number +): ExternalAutomationRunTableState { + return { + ...resolveExternalAutomationRunTableState(state, job), + page, + selectedRunId: null + } +} + +export function resolveExternalAutomationFetchedRuns( + state: ExternalAutomationRunTableState, + job: ExternalAutomationRunTableJob, + result: ExternalAutomationRunPageResult +): ExternalAutomationRunTableState { + const resolved = resolveExternalAutomationRunTableState(state, job) + const selectedRunId = + resolved.selectedRunId && result.runs.some((run) => run.id === resolved.selectedRunId) + ? resolved.selectedRunId + : (result.runs[0]?.id ?? null) + + return { + ...resolved, + fetchedRuns: result.runs, + fetchedTotalCount: result.totalCount ?? null, + selectedRunId + } +}