perf(DesignerV2): Workflow selector performance improvements#9023
Merged
perf(DesignerV2): Workflow selector performance improvements#9023
Conversation
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | Keep as-is; optionally include a small detail about the sub-systems affected |
| Commit Type | ✅ | No change needed |
| Risk Level | Recommend updating to Medium or expand PR body to justify Low with mitigations and rollout plan |
|
| What & Why | ✅ | Good — consider noting dependency handling for yocto-queue |
| Impact of Change | ✅ | Add a short bullet of the exact selectors/components to smoke test |
| Test Plan | ✅ | Tests are present in diff — add brief manual test steps to the body for clarity |
| Contributors | ✅ | OK |
| Screenshots/Videos | ✅ | N/A is reasonable |
Final Notes
- Overall this PR body and title are high quality and the template is filled correctly. Unit tests were added and I validated their presence in the code diff.
- My advised risk level is medium (higher than the PR's current Low). Reason: core selector/hook changes and state-shape/memoization adjustments impact many render paths — even small mistakes can cause subtle regressions (incorrect memoization keys, selector cache leakage, or missing dependency in package.json). Please either bump the risk label to
risk:mediumor expand the PR body to explain why this truly is Low risk and what mitigations/tests/staged rollout are in place.
Recommended next steps (actionable):
- Update the PR body Risk Level checkbox to Medium (or add a justification paragraph for keeping Low). Ensure label matches.
- Add a short "Manual Testing Performed" bullet list describing how you exercised the changes (example: opened a 1000-node workflow, toggled collapsed graphs, validated OperationCard/ScopeCard rendering and repetition names, etc.).
- Confirm whether
yocto-queuerequires package.json changes; if so, include them and confirm CI passes. If the dependency already exists, state that in the PR body. - Optionally add a smoke-test checklist for reviewers to run locally or in a staging environment.
Please update the PR title/body (risk label or justification and manual test steps) and re-submit. Thank you for the thorough work and tests — these performance improvements look valuable.
Last updated: Tue, 07 Apr 2026 22:33:45 GMT
📊 Coverage CheckThe following changed files need attention:
Please add tests for the uncovered files before merging. |
Contributor
There was a problem hiding this comment.
Pull request overview
Performance-focused update to DesignerV2 workflow graph selection and node rendering, aimed at reducing hot-path graph traversal cost and preventing unnecessary React re-renders in per-node card components.
Changes:
- Added a memoized workflow node index (Map) to make
useWorkflowNodeO(1) per lookup and fixed DFS short-circuit behavior in graph traversal helpers. - Reduced repeated O(n) membership checks by switching
Array.includes/.somepatterns toSet.hasin selector/hook loops. - Narrowed metadata subscriptions in node card components via a new
useRepetitionNamehook, and updateduseHandoffEdgesto a selector-based pattern.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/designer-v2/src/lib/ui/CustomNodes/ScopeCardNode.tsx | Replaces broad nodesMetadata subscription with useRepetitionName to reduce cascading re-renders. |
| libs/designer-v2/src/lib/ui/CustomNodes/OperationCardNode.tsx | Same narrowing of metadata subscription via useRepetitionName. |
| libs/designer-v2/src/lib/ui/CustomNodes/test/ScopeCardNode.spec.tsx | Updates mocks to align with useRepetitionName usage. |
| libs/designer-v2/src/lib/ui/CustomNodes/test/OperationCardNode.spec.tsx | Updates mocks to align with useRepetitionName usage. |
| libs/designer-v2/src/lib/ui/common/LoopsPager/helper.ts | Introduces useRepetitionName hook that selects a primitive repetition name string from state. |
| libs/designer-v2/src/lib/core/state/workflow/workflowSelectors.ts | Adds memoized node index selector, optimizes membership checks, fixes traversal short-circuiting, and refactors useHandoffEdges. |
| libs/designer-v2/src/lib/core/state/workflow/test/workflowNodeIndex.spec.ts | Adds unit tests for traversal/indexing logic (currently via inlined implementations). |
libs/designer-v2/src/lib/core/state/workflow/workflowSelectors.ts
Outdated
Show resolved
Hide resolved
libs/designer-v2/src/lib/core/state/workflow/workflowSelectors.ts
Outdated
Show resolved
Hide resolved
libs/designer-v2/src/lib/core/state/workflow/__test__/workflowNodeIndex.spec.ts
Outdated
Show resolved
Hide resolved
ccastrotrejo
approved these changes
Apr 7, 2026
This was referenced Apr 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Type
Risk Level
What & Why
Optimize hot-path graph traversals and reduce unnecessary React re-renders in the designer-v2 workflow engine.
Three categories of improvements:
O(1) node lookups via memoized index — useWorkflowNode previously called getWorkflowNodeFromGraphState, which performed a recursive DFS on every invocation. Added a buildNodeIndex (BFS) + selectNodeIndex (createSelector memoized on graph reference) that builds a Map<string, WorkflowNode> once per graph change. useWorkflowNode now does a single Map.get(). Also fixed a short-circuit bug in both getWorkflowNodeFromGraphState and getWorkflowGraphPath where the traversal continued iterating siblings after finding the target node.
Array.includes → Set.has in loops — Converted useNewAdditiveSubgraphId (.some() over all node IDs) and useDisconnectedNodes (.includes() inside .filter()) to use Set for O(1) membership checks. Deliberately skipped filterOutNodeIdsRecursive where the input array is typically 0–3 items and Set construction overhead would exceed any benefit.
Narrowed selectors to prevent cascading re-renders — ScopeCardNode and OperationCardNode (rendered for every node in the graph) subscribed to the full nodesMetadata dictionary via useNodesMetadata(), causing all nodes to re-render on any metadata change. Replaced with a new useRepetitionName hook that returns a string primitive, so React's === comparison short-circuits re-renders. Also converted useHandoffEdges from useNodesMetadata() + useMemo to a createSelector-based pattern.
Impact of Change
Users: No behavior changes. Workflows with many nodes should feel more responsive during editing and monitoring.
Developers: New useRepetitionName hook available in LoopsPager/helper.ts. useWorkflowNode is now O(1). selectNodeIndex available for any future per-node selectors.
System: Reduced graph traversal from O(n) to O(1) per node lookup (~11.7x speedup measured in benchmark). Fewer React re-renders in per-node card components.
Test Plan
Contributors
@rllyy97
Screenshots/Videos
N/A