Skip to content

feat: Vue node viewport virtualization and low-zoom level of detail to reduce DOM and rendering work in large Nodes 2.0 workflows - #14119

Closed
r-vage wants to merge 13 commits into
Comfy-Org:mainfrom
r-vage:research/viewport-virtualization-4916efd7f
Closed

feat: Vue node viewport virtualization and low-zoom level of detail to reduce DOM and rendering work in large Nodes 2.0 workflows#14119
r-vage wants to merge 13 commits into
Comfy-Org:mainfrom
r-vage:research/viewport-virtualization-4916efd7f

Conversation

@r-vage

@r-vage r-vage commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds experimental Vue node viewport virtualization and low-zoom level of detail to reduce DOM and rendering work in large Nodes 2.0 workflows while preserving layout and active interactions.

Changes

  • What: Mounts nodes once for hydration, then renders only nodes within the settled viewport when virtualization is enabled.
  • Keeps focused, selected, dragged, resized, input-capturing, and link-drag nodes mounted during active interactions.
  • Preserves cached slot geometry while nodes are virtualized and invalidates stale offsets after resizing.
  • Adds configurable low-detail rendering below a zoom threshold while retaining node shells, titles, sockets, links, and layout.
  • Gates virtualization work on both the experimental setting and Vue node rendering mode.
  • Adds unit and browser coverage for hydration, viewport changes, interaction protection, slot tracking, resize-then-move invalidation, auto-pan, pasted nodes, and zoom thresholds.

Review Focus

  • Viewport refresh timing after canvas transforms settle.
  • Interaction protection and slot-layout correctness while nodes are unmounted.
  • Both features are isolated to Vue nodes; viewport virtualization remains disabled by default.

CodeRabbit follow-up review completed with no remaining findings.

Testing

  • pnpm typecheck
  • pnpm typecheck:browser
  • Related Vitest suites: 23 tests passed across viewport virtualization, slot tracking, and low-zoom LOD.
  • ESLint and oxfmt checks passed for all changed files.
  • Playwright successfully collected all 6 viewport virtualization scenarios.
  • E2E regression coverage for the fix-labeled commits: browser_tests/tests/vueNodes/viewportVirtualization.spec.ts

@r-vage
r-vage requested a review from a team July 26, 2026 19:53
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jul 26, 2026
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

🎨 Storybook: 🚧 Building...

🎭 Playwright: ⏳ Running...

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1a27b06e-226f-44cd-9494-629cbe00abf1

📥 Commits

Reviewing files that changed from the base of the PR and between b53a4b2 and cf5ac4f.

📒 Files selected for processing (3)
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/schemas/apiSchema.ts

📝 Walkthrough

Walkthrough

Vue node rendering now supports opt-in viewport virtualization, interaction-aware hydration, persistent slot layouts, zoom-based low-detail rendering, compact collapsed-node presentation, and related unit and browser tests.

Changes

Vue node rendering

Layer / File(s) Summary
Settings and rendering integration
src/schemas/apiSchema.ts, src/platform/settings/constants/coreSettings.ts, src/locales/en/settings.json, src/components/graph/GraphCanvas.vue, src/renderer/extensions/vueNodes/components/LGraphNode.vue, browser_tests/fixtures/*
Adds Vue Nodes settings, wires renderedNodes and mount notifications into canvas rendering, updates collapsed-node headers and sizing, and extends browser-test helpers.
Viewport virtualization engine
src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts, src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts, src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts, browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
Tracks hydrated, visible, and protected nodes; refreshes viewport coverage after settled canvas transforms; preserves active interaction nodes; and validates virtualization through unit and E2E tests.
Virtualized slot-layout lifecycle
src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts, src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts, src/composables/graph/useGraphNodeManager.ts, src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
Retains slot entries across virtualized unmounts, invalidates cached geometry on movement or resize, and clears tracked layouts on node removal and manager disposal.
Low-detail rendering and validation
src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts, src/assets/css/style.css, src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts, browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
Toggles low-detail mode from canvas zoom, hides selected node content and widgets through CSS, and tests threshold behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GraphCanvas
  participant useViewportVirtualization
  participant LGraphNode
  participant Canvas
  GraphCanvas->>useViewportVirtualization: pass graph nodes and virtualization setting
  useViewportVirtualization-->>GraphCanvas: return renderedNodes
  GraphCanvas->>LGraphNode: mount rendered node
  LGraphNode->>useViewportVirtualization: emit node-mounted(nodeId)
  Canvas->>useViewportVirtualization: report settled transform
  useViewportVirtualization-->>GraphCanvas: update viewport node set
Loading

Possibly related PRs

Suggested reviewers: austinmroz

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly captures the main change: Vue node viewport virtualization and low-detail rendering for large Nodes 2.0 workflows.
Description check ✅ Passed The description matches the template well, covering summary, changes, review focus, and testing; screenshots are optional and no breaking/dependencies exist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
End-To-End Regression Coverage For Fixes ✅ Passed The PR includes browser_tests/ coverage, so it doesn’t meet the fail criteria even if it were fix-related.
Adr Compliance For Entity/Litegraph Changes ✅ Passed Changed files are Vue-node components/composables/tests, not core entity classes; no direct node.pos/size/group mutations or new entity methods/callbacks appear in the diff.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@browser_tests/tests/vueNodes/viewportVirtualization.spec.ts`:
- Around line 120-122: Update the virtualization test around vueNodes.getNodeIds
so it waits for the 150 ms settle side-effect, such as the offset becoming
settled, before asserting the node IDs once. Remove the immediate expect.poll on
getNodeIds and preserve the expected [nodeId] assertion after the settle window
to verify the mount set remains frozen.

In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts`:
- Around line 373-402: Add an assertion to the test case “uses a matching node
container when a foreign slot entry comes first” verifying that
layoutStore.getSlotLayout(foreignSlotKey) is undefined or otherwise absent after
syncNodeSlotLayoutsFromDOM(NODE_ID), while preserving the existing matching-slot
position assertion.

In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts`:
- Around line 370-374: The virtualization-clearing flow must reconcile retained
entries whose elements were unset, rather than leaving their stopWatch and
cached layouts active after clearViewportVirtualizedNodeIds(). Update the
relevant useSlotElementTracking logic to prune entries no longer present in
virtualizedNodeIds or expand them through the normal synchronization path, while
preserving active entries and preventing stale updateNodeSlotsFromCache calls
without DOM elements.

In
`@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts`:
- Around line 21-51: Update the hoisted useRafFn mock so its returned isActive
is a reactive Ref<boolean>, such as a shallowRef, rather than a plain object
with a one-time value. Keep rafWatcher.pause and rafWatcher.resume synchronized
with that ref when toggling active state, preserving the existing immediate
initialization behavior.
- Around line 111-163: Expand the virtualization tests around
useViewportVirtualization to assert the published isNodeViewportVirtualized
state after the queued refresh settles, including the focused node’s virtualized
status. Add enabled/disabled boundary coverage verifying the state is cleared
when virtualization is disabled, and assert it is also cleared after scope
disposal; exercise the existing replaceViewportVirtualizedNodeIds publication
path rather than only checking renderedNodes.

In `@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts`:
- Around line 147-158: Ensure mutations to hydratedNodeIds and
pendingHydrationNodeIds in the watcher also invalidate renderedNodes by
incrementing hydrationVersion after the deletions. Preserve the existing void
hydrationVersion.value dependency in renderedNodes and verify every mutation
path updates the version.
- Around line 302-321: Update the disabled branch of the enabled-state watcher
to cancel any pending refreshFrame and hydrationFrame callbacks in addition to
settleTimer, resetting their handles consistently with the cleanup performed in
onScopeDispose before rebuilding the full viewport node set.
- Around line 132-134: Normalize link.node.id before adding it to result in the
activeCanvas renderLinks branch, matching the existing capturingNode.id handling
in the surrounding viewport virtualization logic. Preserve the null check and
ensure numeric IDs are converted to the same string representation used by the
protection lookup.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2e6578f6-8af4-4821-af13-f95f5d609a28

📥 Commits

Reviewing files that changed from the base of the PR and between a98cfa3 and ecbae00.

📒 Files selected for processing (17)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/assets/css/style.css
  • src/components/graph/GraphCanvas.vue
  • src/composables/graph/useGraphNodeManager.ts
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts
  • src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts
  • src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts
  • src/schemas/apiSchema.ts

Comment thread browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
@r-vage
r-vage force-pushed the research/viewport-virtualization-4916efd7f branch from ecbae00 to e16ce89 Compare July 26, 2026 20:06
@vercel

vercel Bot commented Jul 26, 2026

Copy link
Copy Markdown

@r-vage is attempting to deploy a commit to the comfy product design Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/assets/css/style.css`:
- Around line 19-55: Scope the `.dom-widget` selector in the
`html.vue-nodes-low-detail` rule to `
.lg-node[data-node-id]:not([data-node-id^='preview-'])`, matching the adjacent
node-scoped selectors. Preserve the existing hidden and non-interactive behavior
only for non-preview Vue node widgets, not unrelated document-wide widgets.

In `@src/platform/settings/constants/coreSettings.ts`:
- Around line 1204-1244: Update the settings entries identified by
Comfy.VueNodes.ViewportVirtualization, Comfy.VueNodes.LowZoomLOD, and
Comfy.VueNodes.FullDetailZoom to use vue-i18n keys instead of inline English
name and tooltip literals. Add the corresponding English translations to
src/locales/en/main.json and resolve them through the repository’s existing i18n
path, preserving the current wording and setting behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 212ec936-edc7-4a0b-a458-0bb878545c0c

📥 Commits

Reviewing files that changed from the base of the PR and between ecbae00 and e16ce89.

📒 Files selected for processing (17)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/assets/css/style.css
  • src/components/graph/GraphCanvas.vue
  • src/composables/graph/useGraphNodeManager.ts
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts
  • src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts
  • src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts
  • src/schemas/apiSchema.ts

Comment thread src/assets/css/style.css
Comment thread src/platform/settings/constants/coreSettings.ts
@r-vage

r-vage commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts`:
- Around line 5-21: Add assertions to the shouldUseVueNodeLowDetail tests
covering NaN, Infinity, and -Infinity canvas scale inputs, verifying each
returns false even when both settings are enabled. Keep the existing threshold
and settings coverage unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ea065018-ad8f-4079-b37e-c4a07f75d1f4

📥 Commits

Reviewing files that changed from the base of the PR and between a98cfa3 and 4f06df4.

📒 Files selected for processing (17)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/assets/css/style.css
  • src/components/graph/GraphCanvas.vue
  • src/composables/graph/useGraphNodeManager.ts
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts
  • src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts
  • src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts
  • src/schemas/apiSchema.ts

Comment thread src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
@r-vage

r-vage commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes.

@r-vage

r-vage commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@browser_tests/tests/vueNodes/viewportVirtualization.spec.ts`:
- Around line 87-90: Update the viewport virtualization test around the offset
polling and getNodeIds assertion to wait beyond the 150 ms settle delay before
sampling node IDs. Follow the existing group-drag test’s page.clock-based timing
approach, then assert the frozen node set after the delayed unmount window.

In
`@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts`:
- Around line 128-139: Clear the shared virtualized-node-id module state in the
beforeEach setup of the viewport virtualization test suite, using the same reset
approach as useSlotElementTracking.test.ts. Ensure each test starts with an
empty viewportVirtualizationState Set, independent of whether a prior test
reached scope.stop(), while preserving the existing rafWatcher and timer
cleanup.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2ea782bf-0b43-4e0c-b9b2-9b674e08fc5c

📥 Commits

Reviewing files that changed from the base of the PR and between a98cfa3 and 45c78f1.

📒 Files selected for processing (17)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/assets/css/style.css
  • src/components/graph/GraphCanvas.vue
  • src/composables/graph/useGraphNodeManager.ts
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts
  • src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts
  • src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts
  • src/schemas/apiSchema.ts

Comment thread browser_tests/tests/vueNodes/viewportVirtualization.spec.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@browser_tests/tests/creditHelpers.spec.ts`:
- Around line 256-258: Replace both icon-based pricing locators in
browser_tests/tests/creditHelpers.spec.ts at lines 256-258 and 328-330 with a
header-scoped exact text locator for “10.6”, using header.getByText; do not
depend on CreditBadge icon CSS classes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b6699c79-ce5e-445e-b5a3-10d834c880b9

📥 Commits

Reviewing files that changed from the base of the PR and between 45c78f1 and bb9bd76.

📒 Files selected for processing (8)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/tests/creditHelpers.spec.ts
  • src/locales/en/settings.json
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/components/NodeHeader.test.ts
  • src/renderer/extensions/vueNodes/components/NodeHeader.vue
  • src/schemas/apiSchema.ts

Comment thread browser_tests/tests/creditHelpers.spec.ts Outdated
@r-vage

r-vage commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts (1)

128-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reset the shared virtualized-node-id module state in beforeEach.

viewportVirtualizationState holds a module-level Set that outlives each test; it is only cleared as a side effect of scope.stop() on the happy path, so any test that throws earlier leaks membership into the next test's isNodeViewportVirtualized assertions. The sibling suite (useSlotElementTracking.test.ts Line 142) already calls clearViewportVirtualizedNodeIds() here.

As per path instructions, .agents/checks/test-quality.md requires ensuring strong isolation (no shared mutable state).

💚 Proposed fix
+import { clearViewportVirtualizedNodeIds } from './viewportVirtualizationState'
+
 describe('viewport virtualization behavior', () => {
   beforeEach(() => {
+    clearViewportVirtualizedNodeIds()
     rafWatcher.callback = undefined
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts`
around lines 128 - 133, Update the test setup beforeEach to reset the
module-level virtualized-node ID state by calling
clearViewportVirtualizedNodeIds(), alongside the existing rafWatcher resets.
Ensure each test starts with an empty viewport virtualization set, including
when a prior test exits before scope.stop().

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts`:
- Around line 101-136: Move protected-node computation out of the unconditional
per-frame path in refreshProtectedNodeIds and cache the result. Refresh the
cache only from the existing relevant event listeners and watches for canvas
interaction state, canvasStore.selectedNodeIds,
titleEditorStore.titleEditorTarget, focused/capturing nodes, and linkDragState;
keep the RAF loop limited to transform comparison and applying the cached set,
while preserving protection updates whenever these inputs change.
- Around line 286-288: Update the watcher around allNodes in
useViewportVirtualization to stop using JSON.stringify as its reactive key. Use
a cheaper node-id comparison mechanism, such as a stable comparable key or an
areNodeIdSetsEqual guard inside the callback, so unrelated node updates do not
allocate and compare a serialized full id list on every re-evaluation.

---

Duplicate comments:
In
`@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts`:
- Around line 128-133: Update the test setup beforeEach to reset the
module-level virtualized-node ID state by calling
clearViewportVirtualizedNodeIds(), alongside the existing rafWatcher resets.
Ensure each test starts with an empty viewport virtualization set, including
when a prior test exits before scope.stop().
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e4ea1e5f-f2f5-4c65-8552-e960e5bdadc5

📥 Commits

Reviewing files that changed from the base of the PR and between a98cfa3 and bb91670.

📒 Files selected for processing (21)
  • browser_tests/fixtures/ComfyPage.ts
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/creditHelpers.spec.ts
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/assets/css/style.css
  • src/components/graph/GraphCanvas.vue
  • src/composables/graph/useGraphNodeManager.ts
  • src/locales/en/settings.json
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/components/NodeHeader.test.ts
  • src/renderer/extensions/vueNodes/components/NodeHeader.vue
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts
  • src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts
  • src/renderer/extensions/vueNodes/composables/viewportVirtualizationState.ts
  • src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts
  • src/schemas/apiSchema.ts

Comment thread src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts Outdated
@r-vage
r-vage force-pushed the research/viewport-virtualization-4916efd7f branch from 2965d32 to adb9bbb Compare July 27, 2026 09:41
@r-vage

r-vage commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts`:
- Around line 338-368: Replace the animationFrames.size assertions in the “does
not reconcile node ids when only node data changes” test with an observable
virtualization outcome after updating title. Avoid asserting RAF scheduling or
mock state; verify the public node virtualization behavior remains unchanged
while preserving the existing setup and title update flow.

In `@src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts`:
- Around line 52-74: Update the test around useVueNodeLOD to assert the
observable low-detail class or rendered LOD state remains correct across
consecutive RAF callbacks with an unchanged NaN scale, rather than asserting
fullDetailZoom call counts. If no stable public behavior can be asserted, remove
this implementation-detail test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f5845113-b428-4069-a420-90143ea69b67

📥 Commits

Reviewing files that changed from the base of the PR and between bb91670 and 5a068fa.

📒 Files selected for processing (7)
  • browser_tests/tests/vueNodes/viewportVirtualization.spec.ts
  • src/locales/en/settings.json
  • src/platform/settings/constants/coreSettings.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.ts

Comment thread src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts Outdated
Comment thread src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts Outdated
@r-vage r-vage changed the title Vue node viewport virtualization and low-zoom level of detail to reduce DOM and rendering work in large Nodes 2.0 workflows feat: Vue node viewport virtualization and low-zoom level of detail to reduce DOM and rendering work in large Nodes 2.0 workflows Jul 27, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts`:
- Around line 56-75: Update the test’s effectScope lifecycle around
useVueNodeLOD by wrapping the callback assertions in try/finally and calling
scope.stop() in the finally block. Preserve the existing assertions while
ensuring watcher cleanup and DOM state reset even when the test fails.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a513889-5945-4bf6-af43-ac85e446472d

📥 Commits

Reviewing files that changed from the base of the PR and between 5a068fa and 9cceafb.

📒 Files selected for processing (2)
  • src/renderer/extensions/vueNodes/composables/useViewportVirtualization.test.ts
  • src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts

Comment thread src/renderer/extensions/vueNodes/composables/useVueNodeLOD.test.ts Outdated
@r-vage
r-vage marked this pull request as draft July 28, 2026 13:18
@r-vage r-vage closed this Jul 28, 2026
@r-vage
r-vage deleted the research/viewport-virtualization-4916efd7f branch July 28, 2026 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant