fix variadic

[mck]

PR Type

Bug fix, Tests

---

Description

• Fixed variadic input handling in connect method.

• Enhanced edge removal logic for variadic inputs.

• Added comprehensive tests for variadic input scenarios.

• Updated changeset to document the fix.

---

Changes walkthrough 📝

	Relevant files
Bug fix	graph.ts Refactor and fix variadic input handling in graph logic    packages/graph-engine/src/graph/graph.ts Refactored connect method to handle variadic inputs robustly. Added logic to manage placeholders and edge shifting for variadic indices. Improved edge removal logic to handle variadic inputs and maintain consistency. Simplified and optimized edge creation and propagation. +196/-108
Tests	variadic.test.ts Add tests for variadic input handling                                        packages/graph-engine/tests/suites/graph/variadic.test.ts Added tests for variadic input edge cases. Verified behavior for negative indices, appending, and placeholder filling. Tested edge shifting and removal for variadic inputs. +130/-0
Documentation	fluffy-cups-deny.md Add changeset for variadic input fix                                          .changeset/fluffy-cups-deny.md Documented the fix for variadic input handling. +5/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[changeset-bot]

🦋 Changeset detected

Latest commit: 5673274

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@tokens-studio/graph-engine	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The logic for handling variadicIndex in the connect method should be reviewed to ensure edge cases, such as negative indices or large indices, are handled correctly without unintended side effects. variadicIndex = -1 ): Edge { if (targetHandle.variadic) { // negative => error (except -1 which means append) if (variadicIndex < -1) { throw new Error('Invalid variadic index'); } // existing edges const existingEdges = this.inEdges(target.id, targetHandle.name); // sort existingEdges.sort((a, b) => { const av = a.annotations[annotatedVariadicIndex] ?? 0; const bv = b.annotations[annotatedVariadicIndex] ?? 0; return av - bv; }); // if -1 => append if (variadicIndex === -1) { variadicIndex = existingEdges.length; } // if larger => fill placeholders while (existingEdges.length < variadicIndex) { const placeholderIndex = existingEdges.length; const placeholderEdge = new Edge({ id: uuid(), source: '__placeholder__', sourceHandle: '', target: target.id, targetHandle: targetHandle.name, annotations: { [annotatedVariadicIndex]: placeholderIndex } }); // create placeholder existingEdges.push(placeholderEdge); targetHandle._edges.push(placeholderEdge); // fill the array with 0 const arr = Array.isArray(targetHandle.value) ? [...targetHandle.value] : []; arr[placeholderIndex] = 0; targetHandle.setValue(arr, { noPropagate: true }); } // shift existing edges >= variadicIndex existingEdges.forEach(e => { const idx = e.annotations[annotatedVariadicIndex] ?? 0; if (idx >= variadicIndex) { e.annotations[annotatedVariadicIndex] = idx + 1; this.emit('edgeIndexUpdated', e); } }); // Create the new edge first const annotations: VariadicEdgeData = { index: variadicIndex }; const newEdge = this.createEdge({ id: uuid(), source: source.id, target: target.id, sourceHandle: sourceHandle.name, targetHandle: targetHandle.name, annotations: { [annotatedVariadicIndex]: annotations.index }, noPropagate: true // Important: prevent double array update }); // Now update the array values - do this only once const arr = Array.isArray(targetHandle.value) ? [...targetHandle.value] : []; // Make space for the new value by shifting values right for (let i = arr.length - 1; i >= variadicIndex; i--) { arr[i + 1] = arr[i]; } // Insert the new value arr[variadicIndex] = sourceHandle.value; targetHandle.setValue(arr, { noPropagate: true, type: { type: 'array', items: sourceHandle.type } }); // resort and done targetHandle._edges.sort((a, b) => { const av = a.annotations[annotatedVariadicIndex] ?? 0; const bv = b.annotations[annotatedVariadicIndex] ?? 0; return av - bv; }); // Now we can propagate this.propagate(source.id); return newEdge; } else { // if not variadic, only 1 if (targetHandle._edges.length > 0) { throw new Error( `Input ${targetHandle.name} on node ${target.id} is already connected` ); } // create normal edge const newEdge = this.createEdge({ id: uuid(), source: source.id, target: target.id, sourceHandle: sourceHandle.name, targetHandle: targetHandle.name }); return newEdge; } Edge Removal Logic The removeEdge method includes complex logic for handling variadic inputs and shifting indices. Ensure this logic is robust and does not introduce inconsistencies in the graph state. removeEdge(edgeId: string): boolean { const e = this.edges[edgeId]; if (!e) return false; const sourceNode = this.getNode(e.source); const targetNode = this.getNode(e.target); if (sourceNode && this.successorNodes[e.source]) { this.successorNodes[e.source] = this.successorNodes[e.source].filter( x => x !== e.target ); } // remove from target if (targetNode) { const input = targetNode.inputs[e.targetHandle]; if (input) { if (input.variadic) { // find removed index const removedIndex = e.annotations[annotatedVariadicIndex] ?? 0; // sort edges first input._edges.sort((a, b) => { const av = a.annotations[annotatedVariadicIndex] ?? 0; const bv = b.annotations[annotatedVariadicIndex] ?? 0; return av - bv; }); // remove input._edges = input._edges.filter(ed => ed.id !== edgeId); // remove from array const arr = Array.isArray(input.value) ? [...input.value] : []; if (removedIndex < arr.length) { arr.splice(removedIndex, 1); // Get type from remaining edges const remainingEdge = input._edges[0]; const type = remainingEdge ? this.getNode(remainingEdge.source)?.outputs[ remainingEdge.sourceHandle ].type : input.type?.items || AnySchema; input.setValue(arr, { noPropagate: true, type: { type: 'array', items: type } }); } // shift indexes > removedIndex input._edges.forEach(ed => { const idx = ed.annotations[annotatedVariadicIndex] ?? 0; if (idx > removedIndex) { ed.annotations[annotatedVariadicIndex] = idx - 1; this.emit('edgeIndexUpdated', ed); } }); input._edges.sort((a, b) => { const av = a.annotations[annotatedVariadicIndex] ?? 0; const bv = b.annotations[annotatedVariadicIndex] ?? 0; return av - bv; }); } else { input._edges = input._edges.filter(x => x.id !== edgeId); } } } // remove from source if (sourceNode) { const outp = sourceNode.outputs[e.sourceHandle]; if (outp) { outp._edges = outp._edges.filter(x => x.id !== edgeId); } } delete this.edges[edgeId]; this.emit('edgeRemoved', edgeId); // optionally propagate from the source if (sourceNode) { this.propagate(sourceNode.id); } return true;

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Ensure sourceHandle.value is defined before inserting it into the array to prevent runtime errors Add a check to ensure that sourceHandle.value is defined before attempting to insert it into the arr array to avoid runtime errors. packages/graph-engine/src/graph/graph.ts [308] +if (sourceHandle.value === undefined) { + throw new Error('Source handle value is undefined'); +} arr[variadicIndex] = sourceHandle.value; Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential runtime error by ensuring sourceHandle.value is defined before use. This is a critical validation step that enhances the code's reliability and prevents unexpected crashes.	9
	Add validation to prevent variadicIndex from exceeding the maximum allowable index for existingEdges Ensure that the variadicIndex value is validated to not exceed the maximum allowable index for existingEdges to prevent out-of-bounds errors. packages/graph-engine/src/graph/graph.ts [248-249] if (variadicIndex === -1) { variadicIndex = existingEdges.length; +} else if (variadicIndex > existingEdges.length) { + throw new Error('Variadic index exceeds allowable range'); } Suggestion importance[1-10]: 8 Why: The suggestion adds a crucial validation to ensure that variadicIndex does not exceed the bounds of existingEdges, which could otherwise lead to runtime errors. This directly improves the robustness of the code.	8
	Validate that input.value is an array before modifying it during edge removal Ensure that input.value is an array before attempting to splice it during edge removal to prevent runtime errors. packages/graph-engine/src/graph/graph.ts [435-437] +if (!Array.isArray(arr)) { + throw new Error('Input value is not an array'); +} if (removedIndex < arr.length) { arr.splice(removedIndex, 1); +} Suggestion importance[1-10]: 7 Why: The suggestion ensures that input.value is an array before attempting to modify it, which prevents potential runtime errors. This is a reasonable safeguard that improves the code's robustness.	7

[SorsOps]

No, lets not do this . Spamming placeholders is not a good idea here. If the issue is that the variadic is causing a problem on load we should rather handle that in the deserialization phase. This will just complicate the system and likely cause more issues

[SorsOps]

I also need a better description of the actual problem we're fixing here, which is not apparent from the code, it mostly seems to just be a lot more verbose and inefficient for some of the operations