perf(mobile): skip re-rendering unchanged worktree rows (#28058)

* perf(mobile): skip re-rendering unchanged worktree rows Rows reach the host list freshly parsed and copied on every changed worktree.ps poll, so the row memo never skipped and every mounted row re-rendered each poll. The row memo now compares the item by value with the catalog's areWorktreesEqual, extended to every Worktree and agent row field, and every other prop by identity. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): compare worktree rows by structural equality The hand-written Worktree field list needed a completeness guard that no compiler checks, since mobile test files sit outside tsc. A generic structural equality covers every field by construction, so a new field can never be skipped and freeze a row. Rows memo on it directly: the item and repo icon compare by value, callbacks and `now` by identity. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
2026-10-30 16:47:34 +00:00 · 2026-10-12 20:54:20 -04:00
parent 92a520c538
commit 0c42285eb2
7 changed files with 243 additions and 188 deletions
@@ -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<Worktree['status']> }

 const stableRepoIcon = { type: 'emoji', emoji: 'o' } as const
 let updateSibling: Dispatch<SetStateAction<number>> = () => undefined
@@ -113,7 +111,11 @@ function agent(overrides: Partial<RuntimeWorktreeAgentRow> = {}): 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 () => {
@@ -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<T extends WorktreeListRowItem> = {
-  item: T
+type Props = {
+  item: Worktree
  isReadOnly: boolean
  now: number
  repoColor: string
@@ -71,12 +38,12 @@ type Props<T extends WorktreeListRowItem> = {
  // 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<T extends WorktreeListRowItem>({
+function WorktreeListRowComponent({
  item,
  isReadOnly,
  now,
@@ -87,7 +54,7 @@ function WorktreeListRowComponent<T extends WorktreeListRowItem>({
  onPress,
  onLongPress,
  onToggleLineage
-}: Props<T>) {
+}: 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<T extends WorktreeListRowItem>({
  )
 }

-export const WorktreeListRow = memo(WorktreeListRowComponent) as typeof WorktreeListRowComponent
+export const WorktreeListRow = memo(WorktreeListRowComponent, areStructurallyEqual)

 const styles = StyleSheet.create({
  worktreeRow: {
@@ -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: <T,>(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> = {}): 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<string>(),
+    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<typeof HostWorkspaceList>[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())
+  })
+})
@@ -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)
+  })
+})
@@ -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))
+    )
+  )
+}
@@ -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)
-  })
 })
@@ -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)
 }