diff --git a/mobile/src/components/WorktreeListRow.test.ts b/mobile/src/components/WorktreeListRow.test.ts index 722202d004b..8111c01cafe 100644 --- a/mobile/src/components/WorktreeListRow.test.ts +++ b/mobile/src/components/WorktreeListRow.test.ts @@ -11,7 +11,8 @@ import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { RuntimeWorktreeAgentRow } from '../../../src/shared/runtime-types' import { WorktreeAgentRow } from './WorktreeAgentRow' -import { WorktreeListRow, type WorktreeListRowItem } from './WorktreeListRow' +import type { Worktree } from '../worktree/workspace-list-types' +import { WorktreeListRow } from './WorktreeListRow' const { agentSpinnerRender, agentStateDotRender } = vi.hoisted(() => ({ agentSpinnerRender: vi.fn(), @@ -56,10 +57,7 @@ vi.mock('./WorktreeMetaGlyphs', () => ({ WorktreeMetaGlyphs: () => null })) -type TestItem = WorktreeListRowItem & { - status: 'working' | 'active' | 'permission' | 'done' | 'inactive' - lastOutputAt: number -} +type TestItem = Worktree & { status: NonNullable } const stableRepoIcon = { type: 'emoji', emoji: 'o' } as const let updateSibling: Dispatch> = () => undefined @@ -113,7 +111,11 @@ function agent(overrides: Partial = {}): RuntimeWorktre const baseItem: TestItem = { worktreeId: 'worktree-1', + repoId: 'repo-1', repo: 'orca', + path: 'mobile-list', + hasAttachedPty: true, + isPinned: false, branch: 'feature/mobile-list', displayName: 'mobile-list', liveTerminalCount: 1, @@ -172,6 +174,41 @@ describe('memoized worktree rows', () => { expect(agentSpinnerRender).toHaveBeenCalledTimes(++expectedRenders) }) + it('skips a value-equal item and re-renders for time, callbacks and nested agent fields', async () => { + const onPress = () => undefined + const row = (item: TestItem, overrides: { now?: number; onPress?: () => void } = {}) => + createElement(WorktreeListRow, { + item, + isReadOnly: false, + now: overrides.now ?? 2_000, + repoColor: '#000000', + repoIcon: { type: 'emoji', emoji: 'o' }, + status: item.status, + onPress: overrides.onPress ?? onPress + }) + await act(async () => { + renderer = create(row(baseItem)) + }) + expect(agentSpinnerRender).toHaveBeenCalledTimes(1) + + // A fresh parse of the same row, with a fresh repo icon object, is not a change. + await act(async () => renderer!.update(row(structuredClone(baseItem)))) + expect(agentSpinnerRender).toHaveBeenCalledTimes(1) + + await act(async () => renderer!.update(row(baseItem, { now: 32_000 }))) + expect(agentSpinnerRender).toHaveBeenCalledTimes(2) + + const nestedChange = structuredClone(baseItem) + nestedChange.agents![0]!.lastAssistantMessage = 'Tests pass' + await act(async () => renderer!.update(row(nestedChange, { now: 32_000 }))) + expect(agentSpinnerRender).toHaveBeenCalledTimes(3) + + await act(async () => + renderer!.update(row(nestedChange, { now: 32_000, onPress: () => undefined })) + ) + expect(agentSpinnerRender).toHaveBeenCalledTimes(4) + }) + it('memoizes agent rows without hiding agent updates', async () => { const firstAgent = agent() await act(async () => { diff --git a/mobile/src/components/WorktreeListRow.tsx b/mobile/src/components/WorktreeListRow.tsx index 0da38990012..c564f6e24fd 100644 --- a/mobile/src/components/WorktreeListRow.tsx +++ b/mobile/src/components/WorktreeListRow.tsx @@ -9,12 +9,12 @@ import { Server } from 'lucide-react-native' import { Pressable, StyleSheet, Text, View } from 'react-native' -import { parseExecutionHostId, type ExecutionHostId } from '../../../src/shared/execution-host' -import type { AgentWorkingMode } from '../../../src/shared/agent-status-types' -import type { RuntimeWorktreeAgentRow } from '../../../src/shared/runtime-types' +import { parseExecutionHostId } from '../../../src/shared/execution-host' import type { MobileRenderableRepoIcon } from '../host-screen/host-screen-reply-schema' import { triggerMediumImpact } from '../platform/haptics' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import type { Worktree } from '../worktree/workspace-list-types' +import { areStructurallyEqual } from '../worktree/structural-equality' import { AgentSpinner } from './AgentSpinner' import { MobileRepoIcon } from './MobileRepoIcon' import { WorktreeAgentList } from './WorktreeAgentList' @@ -26,43 +26,10 @@ function displayBranch(branch: string): string { return branch.replace(/^refs\/heads\//, '') } -// Minimal row shape needed for rendering — a structural subset of the screen's -// Worktree so this component stays decoupled from the screen's local type. -export type WorktreeListRowItem = { - workspaceKind?: 'git' | 'folder-workspace' - worktreeId: string - hostId?: ExecutionHostId - /** Present only when the list spans hosts; names the host this row runs on. */ - hostContextLabel?: string - /** Resolved host for the display label; present when legacy rows omit hostId. */ - hostContextHostId?: ExecutionHostId - /** The host's health word, e.g. "Disconnected"; absent while the host is healthy. */ - hostContextHealthLabel?: string - repo: string - branch: string - displayName: string - path?: string - liveTerminalCount: number - preview: string - unread: boolean - isActive?: boolean - linkedPR: { number: number; state: string } | null - linkedIssue?: number | null - linkedLinearIssue?: string | null - linkedGitLabMR?: number | null - linkedGitLabIssue?: number | null - comment?: string - lineageDepth?: number - lineageChildCount?: number - lineageCollapsed?: boolean - agents?: RuntimeWorktreeAgentRow[] - workingMode?: AgentWorkingMode -} - type WorktreeRollupStatus = 'working' | 'active' | 'permission' | 'done' | 'inactive' -type Props = { - item: T +type Props = { + item: Worktree isReadOnly: boolean now: number repoColor: string @@ -71,12 +38,12 @@ type Props = { // omits its own repo icon+name to avoid the redundant "📁 orca" on every row. hideRepo?: boolean status: WorktreeRollupStatus - onPress: (item: T) => void - onLongPress?: (item: T) => void - onToggleLineage?: (item: T) => void + onPress: (item: Worktree) => void + onLongPress?: (item: Worktree) => void + onToggleLineage?: (item: Worktree) => void } -function WorktreeListRowComponent({ +function WorktreeListRowComponent({ item, isReadOnly, now, @@ -87,7 +54,7 @@ function WorktreeListRowComponent({ onPress, onLongPress, onToggleLineage -}: Props) { +}: Props) { const isFolderWorkspace = item.workspaceKind === 'folder-workspace' const folderMeta = item.comment?.trim() || item.path || 'Folder' const metaText = isFolderWorkspace ? folderMeta : displayBranch(item.branch) @@ -239,7 +206,7 @@ function WorktreeListRowComponent({ ) } -export const WorktreeListRow = memo(WorktreeListRowComponent) as typeof WorktreeListRowComponent +export const WorktreeListRow = memo(WorktreeListRowComponent, areStructurallyEqual) const styles = StyleSheet.create({ worktreeRow: { diff --git a/mobile/src/host-screen/host-workspace-list-row-memo.test.tsx b/mobile/src/host-screen/host-workspace-list-row-memo.test.tsx new file mode 100644 index 00000000000..01b76916ca1 --- /dev/null +++ b/mobile/src/host-screen/host-workspace-list-row-memo.test.tsx @@ -0,0 +1,125 @@ +import { createElement, Fragment, type ReactNode } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { describe, expect, it, vi } from 'vitest' +import type { Section, Worktree } from '../worktree/workspace-list-types' + +const { rowRenders } = vi.hoisted(() => ({ rowRenders: vi.fn() })) + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + RefreshControl: 'RefreshControl', + // Renders every row through the list's own renderItem, as a mounted SectionList would. + SectionList: ({ + sections, + keyExtractor, + renderItem + }: { + sections: Section[] + keyExtractor: (item: Worktree) => string + renderItem: (info: { item: Worktree }) => ReactNode + }) => + sections.flatMap((section) => + section.data.map((item) => + createElement(Fragment, { key: keyExtractor(item) }, renderItem({ item })) + ) + ), + StyleSheet: { create: (styles: T) => styles }, + Text: 'Text', + View: 'View' +})) +vi.mock('lucide-react-native', () => ({})) +vi.mock('../platform/haptics', () => ({ triggerMediumImpact: vi.fn() })) +vi.mock('../components/AgentSpinner', () => ({ AgentSpinner: () => null })) +vi.mock('../components/MobileRepoIcon', () => ({ MobileRepoIcon: () => null })) +vi.mock('../components/WorktreeAgentList', () => ({ WorktreeAgentList: () => null })) +// The row always renders the glyphs, so each call is one row render, named by its comment. +vi.mock('../components/WorktreeMetaGlyphs', () => ({ + prStateColor: () => '#000000', + WorktreeMetaGlyphs: ({ comment }: { comment?: string }) => { + rowRenders(comment) + return null + } +})) +// Off this embedded path; mocked for their native imports. +vi.mock('../components/MobileSearchField', () => ({})) +vi.mock('../components/NewWorkspaceFab', () => ({ FAB_SIZE: 56 })) + +import { buildSections } from '../worktree/workspace-list-sections' +import { HostWorkspaceList } from './host-workspace-list' + +function worktree(id: string, overrides: Partial = {}): Worktree { + return { + worktreeId: `repo-1::${id}`, + repoId: 'repo-1', + repo: 'orca', + branch: `feature/${id}`, + displayName: id, + path: id, + liveTerminalCount: 1, + hasAttachedPty: true, + preview: '$ codex', + unread: false, + isPinned: false, + linkedPR: null, + comment: id, + status: 'active', + ...overrides + } +} + +const noop = () => {} +const stable = { ref: { current: null }, repoColors: new Map(), repoIcons: new Map() } + +function listProps(worktrees: Worktree[]) { + // Each poll re-parses the payload, so rows are fresh objects even when nothing changed. + const polled: Worktree[] = JSON.parse(JSON.stringify(worktrees)) + const filters = { + filterRepoIds: new Set(), + hideSleeping: false, + hideDefaultBranch: false + } + const sections = buildSections(polled, 'name', filters, '', 'none', new Set()) + const controller = { + actions: { openWorktreeSession: noop, openNewWorktreeModal: noop }, + activeWorktreeScroll: { sectionListRef: stable.ref, onScrollToIndexFailed: noop }, + catalog: { refreshing: false, onRefresh: noop }, + connState: 'connected', + displayWorktrees: polled, + embedded: true, + insets: { bottom: 0 }, + isReadOnly: false, + now: 1_000, + reconnectAttempts: 0, + relayRecovery: { pairingRejected: false }, + sectionsResult: { rawSections: sections, sections, uniqueRepoColors: stable.repoColors }, + settings: { activeFilterCount: 0, toggleWorktreeLineage: noop }, + state: { + actionError: '', + setActionTarget: noop, + groupMode: 'none', + repoIconsByName: stable.repoIcons, + search: '', + showSearch: false, + worktreesLoaded: true + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the list reads only the controller fields above on this connected, embedded, error-free path. + return { controller } as unknown as Parameters[0] +} + +describe('host list rows across polls', () => { + it('re-renders only the row whose worktree changed', () => { + const rows = [worktree('a'), worktree('b'), worktree('c')] + let tree: ReactTestRenderer | null = null + act(() => { + tree = create(createElement(HostWorkspaceList, listProps(rows))) + }) + rowRenders.mockClear() + + const changed = [rows[0]!, { ...rows[1]!, preview: 'Running tests' }, rows[2]!] + act(() => tree!.update(createElement(HostWorkspaceList, listProps(changed)))) + + expect(rowRenders.mock.calls.map(([comment]) => comment)).toEqual(['b']) + act(() => tree!.unmount()) + }) +}) diff --git a/mobile/src/worktree/structural-equality.test.ts b/mobile/src/worktree/structural-equality.test.ts new file mode 100644 index 00000000000..dd1ae264b6b --- /dev/null +++ b/mobile/src/worktree/structural-equality.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { areStructurallyEqual } from './structural-equality' + +describe('areStructurallyEqual', () => { + it('equates value-equal nested objects and arrays', () => { + const value = () => ({ id: 'a', agents: [{ state: 'working', mainAgent: { stopping: true } }] }) + expect(areStructurallyEqual(value(), value())).toBe(true) + expect(areStructurallyEqual([1, [2, { x: null }]], [1, [2, { x: null }]])).toBe(true) + }) + + it('detects a changed nested value', () => { + expect( + areStructurallyEqual({ agents: [{ state: 'working' }] }, { agents: [{ state: 'done' }] }) + ).toBe(false) + expect(areStructurallyEqual([1, 2], [1, 2, 3])).toBe(false) + }) + + it('counts keys, so an added or removed key is a change even when undefined', () => { + expect(areStructurallyEqual({ a: 1 }, { a: 1, b: 2 })).toBe(false) + expect(areStructurallyEqual({ a: 1, b: undefined }, { a: 1 })).toBe(false) + expect(areStructurallyEqual({ a: 1 }, { b: 1 })).toBe(false) + }) + + it('compares functions by identity', () => { + const press = () => {} + expect(areStructurallyEqual({ press }, { press })).toBe(true) + expect(areStructurallyEqual({ press }, { press: () => {} })).toBe(false) + }) + + it('does not equate an array with an object of the same entries', () => { + expect(areStructurallyEqual(['a'], { 0: 'a' })).toBe(false) + expect(areStructurallyEqual([], {})).toBe(false) + }) + + it('compares primitives with Object.is', () => { + expect(areStructurallyEqual(Number.NaN, Number.NaN)).toBe(true) + expect(areStructurallyEqual(null, undefined)).toBe(false) + expect(areStructurallyEqual(0, '0')).toBe(false) + }) +}) diff --git a/mobile/src/worktree/structural-equality.ts b/mobile/src/worktree/structural-equality.ts new file mode 100644 index 00000000000..5e59aa88c4b --- /dev/null +++ b/mobile/src/worktree/structural-equality.ts @@ -0,0 +1,23 @@ +// Why: polls re-parse rows into fresh objects, so lists and rows compare by value to skip renders. +export function areStructurallyEqual(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) { + return true + } + if ( + !a || + !b || + typeof a !== 'object' || + typeof b !== 'object' || + Array.isArray(a) !== Array.isArray(b) + ) { + return false + } + const leftEntries = Object.entries(a) + const rightEntries = new Map(Object.entries(b)) + return ( + leftEntries.length === rightEntries.size && + leftEntries.every( + ([key, value]) => rightEntries.has(key) && areStructurallyEqual(value, rightEntries.get(key)) + ) + ) +} diff --git a/mobile/src/worktree/worktree-list-snapshot.test.ts b/mobile/src/worktree/worktree-list-snapshot.test.ts index b575007b002..80d4a7fcd09 100644 --- a/mobile/src/worktree/worktree-list-snapshot.test.ts +++ b/mobile/src/worktree/worktree-list-snapshot.test.ts @@ -222,11 +222,4 @@ describe('areWorktreeListsEqual', () => { expect(areWorktreeListsEqual(first, second)).toBe(false) }) - - it('treats missing and empty agent arrays as equivalent for rendering', () => { - const first = [worktree({ agents: undefined })] - const second = [worktree({ agents: [] })] - - expect(areWorktreeListsEqual(first, second)).toBe(true) - }) }) diff --git a/mobile/src/worktree/worktree-list-snapshot.ts b/mobile/src/worktree/worktree-list-snapshot.ts index 3fe50cec69a..f28198b0adf 100644 --- a/mobile/src/worktree/worktree-list-snapshot.ts +++ b/mobile/src/worktree/worktree-list-snapshot.ts @@ -1,139 +1,9 @@ -import type { RuntimeWorktreeAgentRow } from '../../../src/shared/runtime-types' +import { areStructurallyEqual } from './structural-equality' import type { Worktree } from './workspace-list-sections' export function areWorktreeListsEqual( left: readonly Worktree[], right: readonly Worktree[] ): boolean { - if (left === right) { - return true - } - if (left.length !== right.length) { - return false - } - for (let index = 0; index < left.length; index += 1) { - if (!areWorktreesEqual(left[index]!, right[index]!)) { - return false - } - } - return true -} - -function areWorktreesEqual(left: Worktree, right: Worktree): boolean { - return ( - left.worktreeId === right.worktreeId && - left.repoId === right.repoId && - left.repo === right.repo && - left.branch === right.branch && - (left.hostId ?? null) === (right.hostId ?? null) && - (left.terminalPlatform ?? null) === (right.terminalPlatform ?? null) && - left.displayName === right.displayName && - (left.workspaceStatus ?? null) === (right.workspaceStatus ?? null) && - (left.sortOrder ?? null) === (right.sortOrder ?? null) && - (left.manualOrder ?? null) === (right.manualOrder ?? null) && - (left.lastActivityAt ?? null) === (right.lastActivityAt ?? null) && - (left.createdAt ?? null) === (right.createdAt ?? null) && - left.path === right.path && - (left.isArchived ?? false) === (right.isArchived ?? false) && - (left.isMainWorktree ?? false) === (right.isMainWorktree ?? false) && - (left.hasHostSidebarActivity ?? false) === (right.hasHostSidebarActivity ?? false) && - (left.worktreeInstanceId ?? null) === (right.worktreeInstanceId ?? null) && - (left.lineageWorktreeInstanceId ?? null) === (right.lineageWorktreeInstanceId ?? null) && - (left.parentWorktreeInstanceId ?? null) === (right.parentWorktreeInstanceId ?? null) && - (left.parentWorktreeId ?? null) === (right.parentWorktreeId ?? null) && - areStringArraysEqual(left.childWorktreeIds ?? [], right.childWorktreeIds ?? []) && - left.liveTerminalCount === right.liveTerminalCount && - left.hasAttachedPty === right.hasAttachedPty && - left.preview === right.preview && - left.unread === right.unread && - (left.lastOutputAt ?? null) === (right.lastOutputAt ?? null) && - left.isPinned === right.isPinned && - (left.isActive ?? false) === (right.isActive ?? false) && - (left.linkedIssue ?? null) === (right.linkedIssue ?? null) && - (left.linkedLinearIssue ?? null) === (right.linkedLinearIssue ?? null) && - (left.linkedGitLabMR ?? null) === (right.linkedGitLabMR ?? null) && - (left.linkedGitLabIssue ?? null) === (right.linkedGitLabIssue ?? null) && - (left.comment ?? '') === (right.comment ?? '') && - (left.status ?? null) === (right.status ?? null) && - (left.workingMode ?? null) === (right.workingMode ?? null) && - arePullRequestsEqual(left.linkedPR, right.linkedPR) && - areAgentRowsEqual(left.agents ?? [], right.agents ?? []) - ) -} - -function areStringArraysEqual(left: readonly string[], right: readonly string[]): boolean { - if (left === right) { - return true - } - if (left.length !== right.length) { - return false - } - for (let index = 0; index < left.length; index += 1) { - if (left[index] !== right[index]) { - return false - } - } - return true -} - -function arePullRequestsEqual(left: Worktree['linkedPR'], right: Worktree['linkedPR']): boolean { - if (left === right) { - return true - } - if (!left || !right) { - return false - } - return left.number === right.number && left.state === right.state -} - -function areAgentRowsEqual( - left: readonly RuntimeWorktreeAgentRow[], - right: readonly RuntimeWorktreeAgentRow[] -): boolean { - if (left === right) { - return true - } - if (left.length !== right.length) { - return false - } - for (let index = 0; index < left.length; index += 1) { - const a = left[index]! - const b = right[index]! - if ( - a.paneKey !== b.paneKey || - a.parentPaneKey !== b.parentPaneKey || - a.state !== b.state || - a.workingMode !== b.workingMode || - a.agentType !== b.agentType || - a.prompt !== b.prompt || - a.lastAssistantMessage !== b.lastAssistantMessage || - a.toolName !== b.toolName || - a.toolInput !== b.toolInput || - a.interrupted !== b.interrupted || - !areMainAgentsEqual(a.mainAgent, b.mainAgent) || - a.stateStartedAt !== b.stateStartedAt || - a.updatedAt !== b.updatedAt - ) { - return false - } - } - return true -} - -function areMainAgentsEqual( - left: RuntimeWorktreeAgentRow['mainAgent'], - right: RuntimeWorktreeAgentRow['mainAgent'] -): boolean { - if (left === right) { - return true - } - if (!left || !right) { - return false - } - return ( - left.state === right.state && - left.outcome === right.outcome && - left.stateStartedAt === right.stateStartedAt && - left.stopping === right.stopping - ) + return areStructurallyEqual(left, right) }