perf(DesignerV2): Workflow selector performance improvements

[rllyy97]

Commit Type

• feature - New functionality
• fix - Bug fix
• refactor - Code restructuring without behavior change
• perf - Performance improvement
• docs - Documentation update
• test - Test-related changes
• chore - Maintenance/tooling

Risk Level

• Low - Minor changes, limited scope
• Medium - Moderate changes, some user impact
• High - Major changes, significant user/system impact

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

• Unit tests added/updated
• E2E tests added/updated
• Manual testing completed
• Tested in:

Contributors

@rllyy97

Screenshots/Videos

N/A

[github-actions]

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

✅ PR Title

• Current: perf(DesignerV2): Workflow selector performance improvements
• Issue: None — title is clear, scoped (DesignerV2) and describes the nature of change (perf).
• Recommendation: Keep as-is. If you want to be extra explicit you could add a short parenthetical like (node index, selector memoization) but this is optional.

---

✅ Commit Type

• Properly selected (perf).
• Note: Only one commit type is selected which is correct.

---

⚠️ Risk Level

• Assessment: The PR body marks this change as Low risk and the repo label risk:low is present — that matches the PR metadata. However, based on the code diff this touches central workflow selectors and hooks used widely (introduced buildNodeIndex/selectNodeIndex, changed many selectors to createSelector, replaced broad useNodesMetadata subscriptions with more-scoped selectors/hooks, and added an external queue import). These are cross-cutting changes to state/selectors and re-render behavior and therefore carry a broader surface for regressions.
• Recommendation: Consider raising the risk to Medium. At minimum, include an explicit justification in the PR body for why Low is appropriate (e.g., exhaustive test coverage + benchmark numbers + rollout plan). If you keep Low, please call out the specific mitigations (unit tests, targeted manual tests, canary/staged rollout) so reviewers know you've validated potential regressions.

---

✅ What & Why

• Current:
  "Optimize hot-path graph traversals and reduce unnecessary React re-renders in the designer-v2 workflow engine. ... O(1) node lookups via memoized index ... Array.includes → Set.has ... Narrowed selectors to prevent cascading re-renders ..."
• Issue: None — clear description and lists concrete changes and reasoning. You also included a measured speedup (~11.7x) which is excellent.
• Recommendation: Optionally add a short note about the new dependency usage (yocto-queue) and confirm package.json / lockfile changes if any. If no new dependency was added to package.json, mention why (already present, small helper, etc.).

---

✅ Impact of Change

• Impact section present and appropriate.
• Recommendation: Expand slightly to call out specifically which subsystems/components are most likely to be affected (e.g., "selectors under libs/designer-v2/src/lib/core/state/workflow — useWorkflowNode, useHandoffEdges, useRootWorkflowGraphForLayout — are changed and should be smoke-tested"). This helps reviewers and release managers focus testing.
  • Users: No behavior changes expected, improved responsiveness for large graphs.
  • Developers: New hooks/selectors: useRepetitionName, selectNodeIndex; changes to useWorkflowNode logic.
  • System: Reduced traversal cost from O(n) to O(1) per lookup and fewer React re-renders.

---

✅ Test Plan

• Assessment: Unit tests are checked in and visible in the diff (several new/updated test files). Manual testing is checked in the PR body. E2E tests are not included (unchecked) but that's acceptable if you provide a rationale; you did mark manual testing completed.
• Recommendation: Add a short note listing the test scenarios you manually validated (e.g., "opened large workflow with N nodes, added/removed nodes, verified no incorrect renders, validated repetition name formatting in loops") so reviewers know what manual checks were executed.

---

✅ Contributors

• Assessment: Contributors list present (@rllyy97). Good.
• Recommendation: If other teammates reviewed/ran tests, consider listing them here.

---

✅ Screenshots/Videos

• Assessment: N/A — appropriate for a non-visual perf change.
• Recommendation: None.

---

Summary Table

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:medium or 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):

1. Update the PR body Risk Level checkbox to Medium (or add a justification paragraph for keeping Low). Ensure label matches.
2. 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.).
3. Confirm whether yocto-queue requires package.json changes; if so, include them and confirm CI passes. If the dependency already exists, state that in the PR body.
4. 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

[github-actions]

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/state/workflow/workflowSelectors.ts - 24% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/ui/CustomNodes/ScopeCardNode.tsx - 79% covered (needs improvement)

Please add tests for the uncovered files before merging.

[Copilot]

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 useWorkflowNode O(1) per lookup and fixed DFS short-circuit behavior in graph traversal helpers.
• Reduced repeated O(n) membership checks by switching Array.includes/.some patterns to Set.has in selector/hook loops.
• Narrowed metadata subscriptions in node card components via a new useRepetitionName hook, and updated useHandoffEdges to 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).

[Copilot]

PR description mentions selectNodeIndex being available for reuse, but it's currently a file-local constant. If other selectors/hooks are expected to reuse it, consider exporting it (or exporting a getNodeFromIndex selector) or update the PR description to match the current API surface.