Skip to content

Conversation

@webfiltered
Copy link
Contributor

@webfiltered webfiltered commented Sep 8, 2025

Summary

Changes

  • Resolves logic issue from last-minute refactor in original design (find => findIndex).

Review Focus

Requires actual in-app testing with custom nodes before merge. May or may not do what it says on the tin.

┆Issue is synchronized with this Notion page by Unito

@webfiltered webfiltered requested a review from a team as a code owner September 8, 2025 08:34
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 8, 2025
@github-actions
Copy link

github-actions bot commented Sep 8, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 09/08/2025, 08:45:53 AM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

@AustinMroz
Copy link
Collaborator

AustinMroz commented Sep 8, 2025

The fix is good. Here's a minimal, core only reproduction.
image

The indexing performed prior to the reported error uses Array.at which supports negative indexes. Consequentially, the error can only occur on a node which has no inputs. Virtual nodes generally don't add inputs corresponding to widgets. Since bypassing a subgraph currently also bypasses all inner nodes, any virtual nodes inside a bypassed subgraph (like primitives or a set node) will trigger this bug.

(Though this also suggests that the nodes inside a bypassed subgraph are being explored. This should not be the case, but is a seperate case I've not found a repro for.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dev] ExecutableNodeDTO .#getBypassSlotIndex returns -1 instead of undefined.

4 participants