From e50ca215ca69f1297a6177b16536c3d99e57ffbc Mon Sep 17 00:00:00 2001 From: shrimbly Date: Sat, 28 Feb 2026 22:21:20 +1300 Subject: [PATCH] fix: nodes downstream of converging switch paths incorrectly dimmed The dimming algorithm processed potentially-dimmed nodes in arbitrary order, causing nodes downstream of a rescued convergence point to stay dimmed. Fix by topologically sorting the candidate set (upstream-first) and checking finalDimmed instead of potentiallyDimmed, so rescued nodes properly propagate their active status downstream. Co-Authored-By: Claude Opus 4.6 --- .../utils/__tests__/dimmingUtils.test.ts | 249 ++++++++++++++++++ src/store/utils/dimmingUtils.ts | 64 ++++- 2 files changed, 310 insertions(+), 3 deletions(-) create mode 100644 src/store/utils/__tests__/dimmingUtils.test.ts diff --git a/src/store/utils/__tests__/dimmingUtils.test.ts b/src/store/utils/__tests__/dimmingUtils.test.ts new file mode 100644 index 00000000..fe4fdb15 --- /dev/null +++ b/src/store/utils/__tests__/dimmingUtils.test.ts @@ -0,0 +1,249 @@ +import { describe, it, expect } from "vitest"; +import { computeDimmedNodes } from "../dimmingUtils"; +import type { WorkflowNode, WorkflowEdge } from "@/types"; + +function makeNode(id: string, type: string, data: Record = {}): WorkflowNode { + return { id, type, position: { x: 0, y: 0 }, data } as WorkflowNode; +} + +function makeEdge( + source: string, + target: string, + opts: { sourceHandle?: string; targetHandle?: string } = {} +): WorkflowEdge { + return { + id: `${source}-${target}-${opts.sourceHandle ?? "out"}-${opts.targetHandle ?? "in"}`, + source, + target, + sourceHandle: opts.sourceHandle ?? "image", + targetHandle: opts.targetHandle ?? "image", + } as WorkflowEdge; +} + +function makeSwitchNode( + id: string, + switches: Array<{ id: string; enabled: boolean }> +): WorkflowNode { + return makeNode(id, "switch", { + switches: switches.map(s => ({ ...s, label: s.id })), + }); +} + +function makeConditionalSwitchNode( + id: string, + rules: Array<{ id: string; isMatched: boolean }>, + opts: { evaluationPaused?: boolean } = {} +): WorkflowNode { + return makeNode(id, "conditionalSwitch", { + rules: rules.map(r => ({ ...r, label: r.id })), + evaluationPaused: opts.evaluationPaused ?? false, + }); +} + +describe("computeDimmedNodes", () => { + it("returns empty set when there are no switch nodes", () => { + const nodes = [makeNode("a", "prompt"), makeNode("b", "nanoBanana")]; + const edges = [makeEdge("a", "b", { sourceHandle: "text", targetHandle: "text" })]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.size).toBe(0); + }); + + it("dims nodes downstream of a disabled switch output", () => { + // switch --[disabled]--> gen --> output + const nodes = [ + makeSwitchNode("sw", [{ id: "out1", enabled: false }]), + makeNode("gen", "nanoBanana"), + makeNode("out", "output"), + ]; + const edges = [ + makeEdge("sw", "gen", { sourceHandle: "out1" }), + makeEdge("gen", "out"), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("gen")).toBe(true); + expect(dimmed.has("out")).toBe(true); + }); + + it("does not dim nodes downstream of an enabled switch output", () => { + const nodes = [ + makeSwitchNode("sw", [{ id: "out1", enabled: true }]), + makeNode("gen", "nanoBanana"), + ]; + const edges = [makeEdge("sw", "gen", { sourceHandle: "out1" })]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.size).toBe(0); + }); + + it("rescues a node that has an active input replacing the blocked type (convergence)", () => { + // Two paths from a switch converge on "merge": + // switch --[out1:disabled]--> dimmedPath --> merge + // switch --[out2:enabled]---> activePath --> merge + const nodes = [ + makeSwitchNode("sw", [ + { id: "out1", enabled: false }, + { id: "out2", enabled: true }, + ]), + makeNode("dimmedPath", "nanoBanana"), + makeNode("activePath", "nanoBanana"), + makeNode("merge", "nanoBanana"), + ]; + const edges = [ + makeEdge("sw", "dimmedPath", { sourceHandle: "out1" }), + makeEdge("sw", "activePath", { sourceHandle: "out2" }), + makeEdge("dimmedPath", "merge"), + makeEdge("activePath", "merge"), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("dimmedPath")).toBe(true); + expect(dimmed.has("activePath")).toBe(false); + expect(dimmed.has("merge")).toBe(false); + }); + + it("keeps nodes downstream of a rescued convergence node active (the main bug)", () => { + // switch --[out1:disabled]--> A --> converge --> downstream + // switch --[out2:enabled]---> B --> converge --> downstream + // + // "converge" is rescued because it has active input from B. + // "downstream" must also stay active — it was dimmed before this fix. + const nodes = [ + makeSwitchNode("sw", [ + { id: "out1", enabled: false }, + { id: "out2", enabled: true }, + ]), + makeNode("A", "nanoBanana"), + makeNode("B", "nanoBanana"), + makeNode("converge", "nanoBanana"), + makeNode("downstream", "output"), + ]; + const edges = [ + makeEdge("sw", "A", { sourceHandle: "out1" }), + makeEdge("sw", "B", { sourceHandle: "out2" }), + makeEdge("A", "converge"), + makeEdge("B", "converge"), + makeEdge("converge", "downstream"), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("A")).toBe(true); + expect(dimmed.has("B")).toBe(false); + expect(dimmed.has("converge")).toBe(false); + expect(dimmed.has("downstream")).toBe(false); + }); + + it("keeps a chain of nodes downstream of rescued convergence active", () => { + // switch --[disabled]--> A --> converge --> D1 --> D2 --> D3 + // switch --[enabled]---> B --> converge + const nodes = [ + makeSwitchNode("sw", [ + { id: "out1", enabled: false }, + { id: "out2", enabled: true }, + ]), + makeNode("A", "nanoBanana"), + makeNode("B", "nanoBanana"), + makeNode("converge", "nanoBanana"), + makeNode("D1", "nanoBanana"), + makeNode("D2", "nanoBanana"), + makeNode("D3", "output"), + ]; + const edges = [ + makeEdge("sw", "A", { sourceHandle: "out1" }), + makeEdge("sw", "B", { sourceHandle: "out2" }), + makeEdge("A", "converge"), + makeEdge("B", "converge"), + makeEdge("converge", "D1"), + makeEdge("D1", "D2"), + makeEdge("D2", "D3"), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("A")).toBe(true); + expect(dimmed.has("B")).toBe(false); + expect(dimmed.has("converge")).toBe(false); + expect(dimmed.has("D1")).toBe(false); + expect(dimmed.has("D2")).toBe(false); + expect(dimmed.has("D3")).toBe(false); + }); + + describe("ConditionalSwitch", () => { + it("dims downstream of non-matching rule outputs", () => { + const nodes = [ + makeConditionalSwitchNode("cs", [ + { id: "rule1", isMatched: true }, + { id: "rule2", isMatched: false }, + ]), + makeNode("active", "nanoBanana"), + makeNode("dimmed", "nanoBanana"), + ]; + const edges = [ + makeEdge("cs", "active", { sourceHandle: "rule1" }), + makeEdge("cs", "dimmed", { sourceHandle: "rule2" }), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("active")).toBe(false); + expect(dimmed.has("dimmed")).toBe(true); + }); + + it("dims default output when a rule matches", () => { + const nodes = [ + makeConditionalSwitchNode("cs", [{ id: "rule1", isMatched: true }]), + makeNode("defaultTarget", "nanoBanana"), + ]; + const edges = [ + makeEdge("cs", "defaultTarget", { sourceHandle: "default" }), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("defaultTarget")).toBe(true); + }); + + it("keeps downstream of rescued convergence active (conditionalSwitch variant)", () => { + // condSwitch --[rule1:matched]--> B --> converge --> downstream + // condSwitch --[rule2:unmatched]--> A --> converge --> downstream + const nodes = [ + makeConditionalSwitchNode("cs", [ + { id: "rule1", isMatched: true }, + { id: "rule2", isMatched: false }, + ]), + makeNode("A", "nanoBanana"), + makeNode("B", "nanoBanana"), + makeNode("converge", "nanoBanana"), + makeNode("downstream", "output"), + ]; + const edges = [ + makeEdge("cs", "A", { sourceHandle: "rule2" }), + makeEdge("cs", "B", { sourceHandle: "rule1" }), + makeEdge("A", "converge"), + makeEdge("B", "converge"), + makeEdge("converge", "downstream"), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("A")).toBe(true); + expect(dimmed.has("B")).toBe(false); + expect(dimmed.has("converge")).toBe(false); + expect(dimmed.has("downstream")).toBe(false); + }); + + it("skips dimming entirely when evaluationPaused is true", () => { + const nodes = [ + makeConditionalSwitchNode( + "cs", + [{ id: "rule1", isMatched: false }], + { evaluationPaused: true } + ), + makeNode("target", "nanoBanana"), + ]; + const edges = [ + makeEdge("cs", "target", { sourceHandle: "rule1" }), + ]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.size).toBe(0); + }); + }); + + it("does not dim the switch node itself", () => { + const nodes = [ + makeSwitchNode("sw", [{ id: "out1", enabled: false }]), + makeNode("target", "nanoBanana"), + ]; + const edges = [makeEdge("sw", "target", { sourceHandle: "out1" })]; + const dimmed = computeDimmedNodes(nodes, edges); + expect(dimmed.has("sw")).toBe(false); + }); +}); diff --git a/src/store/utils/dimmingUtils.ts b/src/store/utils/dimmingUtils.ts index b36e09a4..50f609ad 100644 --- a/src/store/utils/dimmingUtils.ts +++ b/src/store/utils/dimmingUtils.ts @@ -68,9 +68,15 @@ export function computeDimmedNodes( // Step 2: Type-aware smart cascade — only un-dim if an active input replaces // the SAME data type that was blocked by the disabled Switch output. // e.g. a Prompt (text) does NOT rescue a node whose image input is disabled. + // + // Process nodes in topological order (upstream first) so that by the time we + // evaluate a node, all its sources have already been resolved into finalDimmed. + // This fixes the convergence bug where a rescued node's downstream was still + // treated as dimmed due to arbitrary iteration order. const finalDimmed = new Set(); + const sortedDimmed = topologicalSort(potentiallyDimmed, edges); - potentiallyDimmed.forEach(nodeId => { + sortedDimmed.forEach(nodeId => { const incomingEdges = edges.filter(e => e.target === nodeId); // Collect which handle types are blocked on this node @@ -105,15 +111,18 @@ export function computeDimmedNodes( blockedTypes.add(edge.targetHandle); } } - } else if (potentiallyDimmed.has(edge.source) && edge.targetHandle) { + } else if (finalDimmed.has(edge.source) && edge.targetHandle) { blockedTypes.add(edge.targetHandle); } }); + // If no handle types are blocked, this node has no reason to be dimmed + if (blockedTypes.size === 0) return; + // Check if any active input provides the same type as a blocked type const hasReplacementInput = incomingEdges.some(edge => { // Skip dimmed sources - if (potentiallyDimmed.has(edge.source)) return false; + if (finalDimmed.has(edge.source)) return false; // Skip disabled Switch outputs const sourceNode = nodes.find(n => n.id === edge.source); if (sourceNode?.type === "switch") { @@ -152,6 +161,55 @@ export function computeDimmedNodes( return finalDimmed; } +/** + * Topological sort of a subset of nodes using Kahn's algorithm. + * Only considers edges between nodes in the subset. + * Returns nodes ordered upstream-first. + */ +function topologicalSort( + nodeIds: Set, + edges: WorkflowEdge[] +): string[] { + // Build in-degree map considering only edges within the subset + const inDegree = new Map(); + nodeIds.forEach(id => inDegree.set(id, 0)); + + const relevantEdges = edges.filter( + e => nodeIds.has(e.source) && nodeIds.has(e.target) + ); + + relevantEdges.forEach(e => { + inDegree.set(e.target, (inDegree.get(e.target) ?? 0) + 1); + }); + + // Start with nodes that have no incoming edges from within the subset + const queue: string[] = []; + inDegree.forEach((deg, id) => { + if (deg === 0) queue.push(id); + }); + + const sorted: string[] = []; + while (queue.length > 0) { + const current = queue.shift()!; + sorted.push(current); + + relevantEdges + .filter(e => e.source === current) + .forEach(e => { + const newDeg = (inDegree.get(e.target) ?? 1) - 1; + inDegree.set(e.target, newDeg); + if (newDeg === 0) queue.push(e.target); + }); + } + + // Append any remaining nodes (cycles) to ensure all are processed + nodeIds.forEach(id => { + if (!sorted.includes(id)) sorted.push(id); + }); + + return sorted; +} + /** * DFS traversal to find all downstream nodes from a starting node. * Uses visited set for cycle detection.