Skip to content

Commit 21f2022

Browse files
committed
fix(workflow): correct two regressions in the canvas perf pass
Gating `toolBlocks` on the picker's open state also emptied it for the always-visible selected-tool chips, which silently fell through to their `getBlock` fallback — the branch documented as the exception for types hidden from the picker. Only `toolGroups`, where the expensive group build lives, is gated now. Re-invoking the find shortcut while the panel was already open stopped re-selecting the query: `open()` is a no-op when the panel is mounted, so the mount-time focus effect never re-ran. The panel publishes its focus callback so the shortcut can drive it either way. A saturated `slice` also allocated a fresh array once the limit covered a whole group, re-rendering the memoized "All blocks" group on every tools page-in — the frame cost the change set out to remove. Alongside those: fold the two reconcilers into one generic and decide reuse by identity rather than a three-write `changed` flag; record why the node comparison is deliberately asymmetric (React Flow augments node objects in place, so a symmetric `isEqual` would never reuse anything); give the browse pagination its own constant instead of borrowing the search-result cap; drop a redundant clamp and the deps it needed; inline the single-consumer `sliceGroupsToLimit`; and split the bundled ref so the hottest component stops allocating an object per render.
1 parent 480294a commit 21f2022

7 files changed

Lines changed: 112 additions & 96 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/connection-block-selector/connection-block-selector.tsx

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ import {
2929
import {
3030
filterAndCap,
3131
GROUP_HEADING_CLASSNAME,
32-
MAX_RESULTS_PER_GROUP,
33-
sliceGroupsToLimit,
3432
} from '@/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/utils'
3533
import {
3634
CMDK_ITEM_GAP_CLASS,
@@ -56,6 +54,7 @@ const SELECTOR_ACTION_MENU_AMPLITUDE = 7
5654
const RECENT_SELECTION_LIMIT = 3
5755
const RECENT_SELECTION_STORAGE_PREFIX = 'sim:connection-block-selector:recent'
5856
const BROWSE_PREFETCH_MARGIN_PX = 640
57+
const BROWSE_PAGE_SIZE = 50
5958
const POPULAR_BLOCK_TYPES = [
6059
'agent',
6160
'function',
@@ -65,6 +64,11 @@ const POPULAR_BLOCK_TYPES = [
6564
'memory',
6665
] as const
6766

67+
/** Ordered prefix that reuses the source array once `count` covers all of it. */
68+
function takePrefix<T>(items: T[], count: number): T[] {
69+
return count >= items.length ? items : items.slice(0, Math.max(0, count))
70+
}
71+
6872
const SELECTOR_PORTS: WorkflowBorderPort[] = [
6973
{
7074
id: 'target',
@@ -151,7 +155,7 @@ export function ConnectionBlockSelector({ id, data }: NodeProps<ConnectionBlockS
151155
const [search, setSearch] = useState('')
152156
const [selectedValue, setSelectedValue] = useState('')
153157
const [recentSelections, setRecentSelections] = useState<RecentSelection[]>([])
154-
const [browseLimit, setBrowseLimit] = useState(MAX_RESULTS_PER_GROUP)
158+
const [browseLimit, setBrowseLimit] = useState(BROWSE_PAGE_SIZE)
155159
const deferredSearch = useDeferredValue(search)
156160
const isSearching = deferredSearch.trim().length > 0
157161
const recentStorageKey = `${RECENT_SELECTION_STORAGE_PREFIX}:${workspaceId}`
@@ -273,30 +277,31 @@ export function ConnectionBlockSelector({ id, data }: NodeProps<ConnectionBlockS
273277
() => availableTools.filter((tool) => !recentSelectionKeys.has(`tool:${tool.id}`)),
274278
[availableTools, recentSelectionKeys]
275279
)
276-
const [visibleBrowseBlocks, visibleBrowseTools] = useMemo(
277-
() => sliceGroupsToLimit([browseBlocks, browseTools], browseLimit),
278-
[browseBlocks, browseLimit, browseTools]
280+
const visibleBrowseBlocks = useMemo(
281+
() => takePrefix(browseBlocks, browseLimit),
282+
[browseBlocks, browseLimit]
283+
)
284+
const visibleBrowseTools = useMemo(
285+
() => takePrefix(browseTools, browseLimit - browseBlocks.length),
286+
[browseBlocks.length, browseLimit, browseTools]
279287
)
280288
const hasMoreBrowseResults = browseLimit < browseBlocks.length + browseTools.length
281289

282290
/**
283-
* Keep the initial commit bounded, then extend the catalog before the user
284-
* reaches its end. Rendering every cmdk item up front caused the original
285-
* frame spike, while a manual pagination control exposed that constraint in
286-
* the UI. Prefetching near the viewport preserves continuous scrolling and
287-
* cmdk's native keyboard navigation without mounting the whole catalog.
291+
* Mounting every cmdk item up front caused the frame spike; a manual "show
292+
* more" control would leak that constraint into the UI. Prefetching near the
293+
* viewport keeps scrolling continuous and cmdk's keyboard navigation intact.
288294
*/
289295
useEffect(() => {
290296
const list = listRef.current
291297
const sentinel = browseSentinelRef.current
292298
if (isSearching || !hasMoreBrowseResults || !list || !sentinel) return
293299

294-
const browseResultCount = browseBlocks.length + browseTools.length
295300
const observer = new IntersectionObserver(
296301
([entry]) => {
297302
if (!entry.isIntersecting) return
298303
startTransition(() => {
299-
setBrowseLimit((current) => Math.min(current + MAX_RESULTS_PER_GROUP, browseResultCount))
304+
setBrowseLimit((current) => current + BROWSE_PAGE_SIZE)
300305
})
301306
},
302307
{
@@ -307,7 +312,7 @@ export function ConnectionBlockSelector({ id, data }: NodeProps<ConnectionBlockS
307312

308313
observer.observe(sentinel)
309314
return () => observer.disconnect()
310-
}, [browseBlocks.length, browseTools.length, hasMoreBrowseResults, isSearching])
315+
}, [hasMoreBrowseResults, isSearching])
311316

312317
const dispatchSelection = useCallback(
313318
(type: string, resultType: 'block' | 'tool' | 'tool_operation', presetOperation?: string) => {

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,9 @@ function UnsupportedToolBadge({ message }: { message: string }) {
464464
)
465465
}
466466

467+
const EMPTY_COMBOBOX_GROUPS: ComboboxOptionGroup[] = []
468+
const EMPTY_COMBOBOX_OPTIONS: ComboboxOption[] = []
469+
467470
export const ToolInput = memo(function ToolInput({
468471
blockId,
469472
subBlockId,
@@ -697,7 +700,6 @@ export const ToolInput = memo(function ToolInput({
697700

698701
const customBlockOverlayVersion = useCustomBlockOverlayVersion()
699702
const toolBlocks = useMemo(() => {
700-
if (!open) return []
701703
const allToolBlocks = getAllBlocks().filter(isAgentToolBlock)
702704
/* An empty option list means the block declares no selectable operation, so
703705
there is nothing to gate — only a wholly denied one leaves the picker. */
@@ -706,7 +708,7 @@ export const ToolInput = memo(function ToolInput({
706708
const { options, denied } = getOperationChoices(block)
707709
return options.length === 0 || options.some((option) => !denied.has(option.id))
708710
})
709-
}, [filterBlocks, customBlockOverlayVersion, getOperationChoices, open])
711+
}, [filterBlocks, customBlockOverlayVersion, getOperationChoices])
710712

711713
const hasBackfilledRef = useRef(false)
712714
useEffect(() => {
@@ -1404,7 +1406,7 @@ export const ToolInput = memo(function ToolInput({
14041406
* @returns Array of option groups for the combobox component
14051407
*/
14061408
const toolGroups = useMemo((): ComboboxOptionGroup[] => {
1407-
if (!open) return []
1409+
if (!open) return EMPTY_COMBOBOX_GROUPS
14081410
const groups: ComboboxOptionGroup[] = []
14091411

14101412
// MCP Server drill-down: when navigated into a server, show only its tools
@@ -1708,7 +1710,7 @@ export const ToolInput = memo(function ToolInput({
17081710
return (
17091711
<div className='w-full space-y-2'>
17101712
<Combobox
1711-
options={[]}
1713+
options={EMPTY_COMBOBOX_OPTIONS}
17121714
groups={toolGroups}
17131715
placeholder='Add tool...'
17141716
/* Every list this picker offers — blocks, operations, MCP and custom

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
3+
import { type RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { Button, cn, Input, toast } from '@sim/emcn'
55
import { ChevronDown, ChevronRight, ChevronUp, X } from '@sim/emcn/icons'
66
import { useParams } from 'next/navigation'
@@ -113,18 +113,27 @@ export function WorkflowSearchReplace() {
113113
const { isOpen, open } = useWorkflowSearchReplaceStore(
114114
useShallow((state) => ({ isOpen: state.isOpen, open: state.open }))
115115
)
116+
const focusSearchInputRef = useRef<(() => void) | null>(null)
116117

117118
useRegisterGlobalCommands([
118119
createCommand({
119120
id: 'open-workflow-search-replace',
120-
handler: open,
121+
handler: () => {
122+
open()
123+
focusSearchInputRef.current?.()
124+
},
121125
}),
122126
])
123127

124-
return isOpen ? <WorkflowSearchReplacePanel /> : null
128+
return isOpen ? <WorkflowSearchReplacePanel focusRef={focusSearchInputRef} /> : null
125129
}
126130

127-
function WorkflowSearchReplacePanel() {
131+
interface WorkflowSearchReplacePanelProps {
132+
/** Lets the shortcut re-select the query while the panel is already open. */
133+
focusRef: RefObject<(() => void) | null>
134+
}
135+
136+
function WorkflowSearchReplacePanel({ focusRef }: WorkflowSearchReplacePanelProps) {
128137
const params = useParams()
129138
const workspaceId = params.workspaceId as string | undefined
130139
const routeWorkflowId = params.workflowId as string | undefined
@@ -180,8 +189,7 @@ function WorkflowSearchReplacePanel() {
180189
setActiveMatchId: state.setActiveMatchId,
181190
}))
182191
)
183-
const prevQueryRef = useRef(query)
184-
const isFirstMatchSyncRef = useRef(true)
192+
const prevQueryRef = useRef<string | null>(null)
185193
const afterReplaceIndexRef = useRef<number | null>(null)
186194
const { data: workspaceCredentials } = useWorkspaceCredentials({ workspaceId })
187195
const { data: customTools = [] } = useCustomTools(workspaceId ?? '')
@@ -265,16 +273,17 @@ function WorkflowSearchReplacePanel() {
265273
)
266274

267275
useEffect(() => {
268-
searchInputRef.current?.focus()
269-
searchInputRef.current?.select()
270-
}, [])
271-
272-
useEffect(
273-
() => () => {
276+
const focusSearchInput = () => {
277+
searchInputRef.current?.focus()
278+
searchInputRef.current?.select()
279+
}
280+
focusSearchInput()
281+
focusRef.current = focusSearchInput
282+
return () => {
283+
focusRef.current = null
274284
usePanelEditorSearchStore.getState().setActiveSearchTarget(null)
275-
},
276-
[]
277-
)
285+
}
286+
}, [focusRef])
278287

279288
const panelHeight = isReplaceExpanded
280289
? SEARCH_PANEL_EXPANDED_HEIGHT
@@ -388,8 +397,6 @@ function WorkflowSearchReplacePanel() {
388397
}
389398

390399
useEffect(() => {
391-
const justOpened = isFirstMatchSyncRef.current
392-
isFirstMatchSyncRef.current = false
393400
const queryChanged = prevQueryRef.current !== query
394401
prevQueryRef.current = query
395402

@@ -404,7 +411,7 @@ function WorkflowSearchReplacePanel() {
404411
const replaceIndex = afterReplaceIndexRef.current
405412
afterReplaceIndexRef.current = null
406413

407-
if (queryChanged || justOpened) {
414+
if (queryChanged) {
408415
handleSelectMatch(hydratedMatches[0].id)
409416
} else if (replaceIndex !== null) {
410417
handleSelectMatch(hydratedMatches[Math.min(replaceIndex, hydratedMatches.length - 1)].id)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-canvas-helpers.ts

Lines changed: 52 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,10 @@ import type { BlockState } from '@/stores/workflows/workflow/types'
77

88
export const SUBFLOW_DROP_TARGET_CLASS = 'subflow-node-drop-target'
99

10-
interface ArrowNavigationEvent {
11-
key: string
12-
repeat: boolean
13-
metaKey: boolean
14-
ctrlKey: boolean
15-
altKey: boolean
16-
shiftKey: boolean
17-
}
10+
type ArrowNavigationEvent = Pick<
11+
KeyboardEvent,
12+
'key' | 'repeat' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey'
13+
>
1814

1915
export function getArrowNavigationDirection(event: ArrowNavigationEvent): -1 | 1 | null {
2016
if (event.repeat || event.metaKey || event.ctrlKey || event.altKey || event.shiftKey) return null
@@ -23,39 +19,62 @@ export function getArrowNavigationDirection(event: ArrowNavigationEvent): -1 | 1
2319
return null
2420
}
2521

22+
/**
23+
* A derivation that changed nothing must produce no new reference at any level,
24+
* so React Flow can skip the subtree. Reuse is decided by identity after
25+
* projection, which also catches a pure reorder: a reused item landing at a
26+
* different index breaks the positional sweep.
27+
*/
28+
function reconcileById<T extends { id: string }>(
29+
current: T[],
30+
derived: T[],
31+
project: (derivedItem: T, currentItem: T | undefined) => T,
32+
isReusable: (currentItem: T, nextItem: T) => boolean
33+
): T[] {
34+
const currentById = new Map<string, T>()
35+
for (const item of current) currentById.set(item.id, item)
36+
37+
const next = derived.map((derivedItem) => {
38+
const currentItem = currentById.get(derivedItem.id)
39+
const nextItem = project(derivedItem, currentItem)
40+
return currentItem && isReusable(currentItem, nextItem) ? currentItem : nextItem
41+
})
42+
43+
const unchanged =
44+
next.length === current.length && next.every((item, index) => item === current[index])
45+
return unchanged ? current : next
46+
}
47+
48+
/**
49+
* Subset comparison, deliberately asymmetric: React Flow writes `width`,
50+
* `height`, `positionAbsolute` and `dragging` onto the node objects it owns, so
51+
* a symmetric `isEqual` against a freshly derived node would never match and no
52+
* node would ever be reused. Only the keys the derivation itself produces are
53+
* compared.
54+
*/
2655
function containsDerivedValues<T extends object>(current: T, derived: T): boolean {
27-
return Object.entries(derived).every(([key, value]) => isEqual(current[key as keyof T], value))
56+
for (const key of Object.keys(derived) as (keyof T)[]) {
57+
if (!isEqual(current[key], derived[key])) return false
58+
}
59+
return true
2860
}
2961

3062
/** Reuses unchanged React Flow node objects while carrying local selection forward. */
3163
export function reconcileCanvasNodes(currentNodes: Node[], derivedNodes: Node[]): Node[] {
32-
const currentById = new Map(currentNodes.map((node) => [node.id, node]))
33-
let changed = currentNodes.length !== derivedNodes.length
34-
const nextNodes = derivedNodes.map((derivedNode, index) => {
35-
const currentNode = currentById.get(derivedNode.id)
36-
const nextNode = { ...derivedNode, selected: currentNode?.selected ?? false }
37-
if (currentNodes[index]?.id !== derivedNode.id) changed = true
38-
if (currentNode && containsDerivedValues(currentNode, nextNode)) return currentNode
39-
changed = true
40-
return nextNode
41-
})
42-
43-
return changed ? nextNodes : currentNodes
64+
return reconcileById(
65+
currentNodes,
66+
derivedNodes,
67+
(derivedNode, currentNode) => ({ ...derivedNode, selected: currentNode?.selected ?? false }),
68+
containsDerivedValues
69+
)
4470
}
4571

46-
/** Reuses unchanged React Flow edge objects after graph-level derivation reruns. */
72+
/**
73+
* Reuses unchanged React Flow edge objects after graph-level derivation reruns.
74+
* Edges carry no React Flow-written fields, so a plain `isEqual` is symmetric-safe.
75+
*/
4776
export function reconcileCanvasEdges(currentEdges: Edge[], derivedEdges: Edge[]): Edge[] {
48-
const currentById = new Map(currentEdges.map((edge) => [edge.id, edge]))
49-
let changed = currentEdges.length !== derivedEdges.length
50-
const nextEdges = derivedEdges.map((derivedEdge, index) => {
51-
const currentEdge = currentById.get(derivedEdge.id)
52-
if (currentEdges[index]?.id !== derivedEdge.id) changed = true
53-
if (currentEdge && isEqual(currentEdge, derivedEdge)) return currentEdge
54-
changed = true
55-
return derivedEdge
56-
})
57-
58-
return changed ? nextEdges : currentEdges
77+
return reconcileById(currentEdges, derivedEdges, (derivedEdge) => derivedEdge, isEqual)
5978
}
6079

6180
/**

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4638,15 +4638,16 @@ const WorkflowContent = React.memo(
46384638
[blocks, getNodes]
46394639
)
46404640

4641+
const latestEdgesRef = useRef(edges)
4642+
latestEdgesRef.current = edges
4643+
const latestBlocksRef = useRef(blocks)
4644+
latestBlocksRef.current = blocks
46414645
/** Stable delete handler to avoid creating new function references per edge. */
4642-
const edgeDeleteStateRef = useRef({ edges, blocks })
4643-
edgeDeleteStateRef.current = { edges, blocks }
46444646
const handleEdgeDelete = useCallback(
46454647
(edgeId: string) => {
4646-
const { edges: currentEdges, blocks: currentBlocks } = edgeDeleteStateRef.current
46474648
// Prevent removing edges targeting protected blocks
4648-
const edge = currentEdges.find((candidate) => candidate.id === edgeId)
4649-
if (edge && isEdgeProtected(edge, currentBlocks)) {
4649+
const edge = latestEdgesRef.current.find((candidate) => candidate.id === edgeId)
4650+
if (edge && isEdgeProtected(edge, latestBlocksRef.current)) {
46504651
toast({ message: 'Cannot remove connections to locked blocks' })
46514652
return
46524653
}
@@ -4777,6 +4778,10 @@ const WorkflowContent = React.memo(
47774778
},
47784779
}
47794780
})
4781+
/* Deliberately impure: referential stability across renders needs the
4782+
previous result, and no state-shaped alternative exists. Safe because
4783+
reconciliation is idempotent — re-running it on its own output returns
4784+
that output by reference, so a discarded render cannot corrupt it. */
47804785
const reconciledEdges = reconcileCanvasEdges(
47814786
previousEdgesWithSelectionRef.current,
47824787
derivedEdges

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/utils.test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,8 @@ import {
1414
scoreActions,
1515
scoreAndSort,
1616
scoreSectionItems,
17-
sliceGroupsToLimit,
1817
} from '@/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/utils'
1918

20-
describe('sliceGroupsToLimit', () => {
21-
it('bounds an ordered browse catalog across groups without reordering it', () => {
22-
const blocks = Array.from({ length: 40 }, (_, index) => `block-${index}`)
23-
const tools = Array.from({ length: 40 }, (_, index) => `tool-${index}`)
24-
25-
expect(sliceGroupsToLimit([blocks, tools], 50)).toEqual([blocks, tools.slice(0, 10)])
26-
expect(blocks).toHaveLength(40)
27-
expect(tools).toHaveLength(40)
28-
})
29-
})
30-
3119
describe('getActionGroupLabel', () => {
3220
const action = {
3321
id: 'test-action',

0 commit comments

Comments
 (0)