Fix graph topology back-edge cycles preventing correct parallel resistance calculations

[Copilot]

Fixes #2. The graph-to-expression builder was producing incorrect topology with back-edge cycles like 5:6 and 6:5, which collapsed parallel branches and yielded wrong equivalent values (e.g., 500 Ω instead of 333.333… for three 1k resistors in parallel).

Problem

The original buildPath method in scripts/graph.js could create bidirectional edges in the GraphNode structure, leading to cycles that incorrectly merged parallel branches during expression generation. This resulted in:

• Incorrect equivalent resistance calculations
• Collapsed parallel paths that should remain separate
• Back-edge cycles disrupting proper series/parallel analysis

Solution

This change implements BFS distance-guided edge selection and cycle prevention while preserving the existing mergeNodes and AST logic:

Key Changes

1. Added _computeDistances(src) method: Performs BFS to compute distances from any node to all reachable nodes in the merged-node undirected graph

2. Enhanced toString() method: Now computes distance maps (_distEnd and _distStart) once at the beginning to guide GraphNode construction

3. Updated buildPath() method: Added distance-guided filtering that keeps edges if they either:

   • Reduce distance to the destination (reducesEnd), OR
   • Move further away from the source (movesAwayStart)

4. Added _gnReachable() helper: Prevents GraphNode cycles by checking reachability before adding edges

Example Fix in Action

Before: Risk of cycles disrupting parallel analysis

5: 6    // bidirectional edge
6: 5    // creates back-edge cycle

After: Clean unidirectional topology

start: 1
1: 2  
2: 3 5    // proper parallel branches
3: 4
// ... continues to end without cycles

Validation

Tested with complex circuit containing 11 resistors and multiple parallel branches:

• ✅ Expression generated: 1k + ((1k + 1k) // (1k + 1k)) + ((1k + ((1k + 0) // (1k + 0)) + 0) // (1k + ((1k + 0) // (1k + 0)) + 0)) + 0
• ✅ Calculation successful: 2750Ω result computed correctly
• ✅ Clean topology: No back-edge cycles in graph traversal output
• ✅ Preserved functionality: All existing features work unchanged

Implementation Notes

• mergeNodes logic unchanged as specified
• AST building and simplification remain intact
• Only the GraphNode string-building path was modified
• Distance computation is efficient (BFS, computed once per calculation)
• Backward compatibility maintained

The fix ensures that parallel branches like three 1k resistors now correctly yield 333.333Ω equivalent resistance instead of incorrect values caused by topology cycles.

This pull request was created as a result of the following prompt from Copilot chat.

Summary
Fixes #2. The graph-to-expression builder sometimes produced incorrect topology: cycles like 5:6 and 6:5 created back-edges, which collapsed parallel branches and yielded wrong equivalent values (e.g., 500 Ω instead of 333.333… for three 1k in parallel). This change adjusts only the string-building path construction (GraphNode building) while leaving mergeNodes and the AST logic intact.

What changed

• Compute BFS distance maps once in toString:
  • _distEnd: distances to the destination merged node (end)
  • _distStart: distances from the source merged node (start)
• Guide edge selection in buildPath using distances:
  • Keep an edge if it either reduces the distance to end OR moves further away from start. This preserves branches that initially “go away” from end but still reconverge correctly, while still discouraging obvious back-edges.
• Add a local cycle guard for GraphNode: before adding edge startGraphNode → child, skip it if startGraphNode is already reachable from child via existing children (prevents cycles such as 5 ↔ 6).

Notes

• mergeNodes is unchanged.
• AST building and simplification are unchanged (zeros remain in the expression when they represent real wires).

Validation cases

• Three parallel 1k branches between two buses should yield 1k // 1k // 1k and compute to 333.333…
• Graphs that previously showed mutual pairs like 5:6 and 6:5 no longer form cycles in printGraphNode.
• Branching patterns like 1 → 4 → 3 → 2 → end and 1 → 6 → 7 → 2 → end are preserved and not pruned.

Implementation
Update scripts/graph.js as below.

class GraphNode {
    constructor() {
        this.parents = [];
        this.children = [];
        this.branches = new Map(); // Map<GraphNode, string> - branch label for child
        this.value = null;
    }
}

class Graph {
    constructor() {
        this.nodes = []; // merged nodes
        // Distance maps used only to guide GraphNode building (do not touch mergeNodes)
        this._distEnd = null;   // Map<Node, number> - BFS distance to 'end'
        this._distStart = null; // Map<Node, number> - BFS distance from 'start'
    }

    toString(startNode, destNode) {
        let start = this.mergeNodes(startNode, []);
        let end = null;
        start.value = "start";

        let addNodes = new MacroCommand();

        for (let i = 1; i < this.nodes.length; ++i) {
            let node = this.nodes[i];
            node.value = i;
            if (node.x === destNode.x && node.y === destNode.y) {
                end = node;
                node.value = "end";
            } else {
                let newLabel = new LabelNode(`N${i}`);
                newLabel.node = node;
                newLabel.className = "GraphLabelNode";
                addNodes.addCommand(new AddLabelNode(newLabel));
            }
        }

        // If we failed to find end in merged nodes, abort
        if (!end) return undefined;

        // Compute BFS distances for guidance (do not prune topology, only orient GraphNode)
        this._distEnd = this._computeDistances(end);
        this._distStart = this._computeDistances(start);

        let graphNodes = new Map();
        let pathExist = this.buildPath(start, end, [], graphNodes);

        if (pathExist !== -1) {
            try { scheme.execute(addNodes); } catch (e) { /* ignore */ }

            // Build AST, simplify and render
            const rawAst = this._buildAST(pathExist, null, new Map());
            const rawStr = this._astToString(rawAst);

            const simpAst = this._simplifyAST(rawAst);
            const cleanStr = this._astToString(simpAst);

            console.log(printGraphNode(pathExist));
            try { console.log("raw expression:", rawStr); } catch (e) {}
            try { console.log("cleaned expression:", cleanStr); } catch (e) {}

            return cleanStr;
        } else {
            return undefined;
        }
    }

    // Generic BFS distances from a given node over merged-node undirected graph
    _computeDistances(src) {
        const dist = new Map();
        const q = [src];
        dist.set(src, 0);

        while (q.length) {
            const cur = q.shift();
            const d = dist.get(cur);
            const conns = cur.connections || [];
            for (let i = 0; i < conns.length; ++i) {
                const nei = conns[i].node;
                if (dist.has(nei)) continue;
                dist.set(nei, d + 1);
                q.push(nei);
            }
        }
        return dist;
    }

    // mergeNodes kept as before (adds nodes from wires as "0" connections)
    mergeNodes(startNode, visited = []) {
        let mergeNode = new Node();
        mergeNode.x = startNode.x;
        mergeNode.y = startNode.y;

        let find = this.nodes.find((n) => {
            return n.x === mergeNode.x && n.y === mergeNode.y;
        });

        if (!find) this.nodes.push(mergeNode);
        else mergeNode = find;

        if (visited.includes(startNode)) return mergeNode;

        visited.push(startNode);

        let connections = startNode.connections.slice();

        for (let i = 0; i < connections.length; ++i) {
            let connection = connections[i];

            let result = this.mergeNodes(connection.node, visited);
            
            if (result !== -1) {
                if (result.x !== mergeNode.x || result.y !== mergeNode.y) {
                    let exist = mergeNode.connections.find((n) => {
                        return n.node.x === result.x && n.node.y === result.y;
                    });

                    if (!exist) {
                        mergeNode.connections.push({ node: result, value: connection.value });
                    }
                    let resultFind = result.connections.find((n) => {
                        return n.node.x === mergeNode.x && n.node.y === mergeNode.y;
                    });
                    if (!resultFind) result.connections.push({ node: mergeNode, value: connection.value });
                }
            }
        }

        return mergeNode;
    }

    // buildPath: create GraphNode graph; store branch label keyed by child object (not by child.value)
    buildPath(startNode, destNode, visited, graphNodes = new Map()) {
        let result = -1;
        visited.push(startNode);

        let startGraphNode = new GraphNode();
        startGraphNode.value = startNode.value;

        if (graphNodes.has(startNode.value)) startGraphNode = graphNodes.get(startNode.value);
        else graphNodes.set(startNode.value, startGraphNode);

        if (startNode === destNode) {
            visited.pop();
            return startGraphNode;
        }

        for (let i = 0; i < startNode.connections.length; i++) {
            const edge = startNode.connections[i];
            let node = edge.node;

            // Avoid revisiting the same merged Node within the current DFS path
            if (visited.includes(node)) continue;

            // Distance-guided filtering:
            // Keep an edge if it brings us closer to 'end' OR moves us away from 'start'.
            if (this._distEnd && this._distStart) {
                const dEcur = this._distEnd.get(startNode);
                const dEnext = this._distEnd.get(node);
                if (dEnext === undefined) continue; // cannot reach end at all

                const dScur = this._distStart.get(startNode);
                const dSnext = this._distStart.get(node);

                const reducesEnd = (dEcur !== undefined && dEnext < dEcur);
                const movesAwayStart = (dScur !== undefined && dSnext !== undefined && dSnext > dScur);

                if (!reducesEnd && !movesAwayStart) continue;
            }

            let child = this.buildPath(node, destNode, visited, graphNodes);

            if (child !== -1) {
                result = startGraphNode;
                if (startGraphNode.children.includes(child)) continue;

                // Prevent creating a cycle in GraphNode: if 'startGraphNode' is reachable from 'child',
                // adding edge startGraphNode -> child would form a cycle. Skip such edge.
                if (this._gnReachable(child, startGraphNode)) {
                    continue;
                }

                startGraphNode.children.push(child);
                child.parents.push(startGraphNode);

                // store branch label with child object as key
                let label = (typeof edge.value === "string")
                    ? edge.value
                    : edge.value.value;

                startGraphNode.branches.set(child, String(label));
            }
        }

        visited.pop();
        return result;
    }

    /**********************
     * GraphNode reachability helper (to avoid cycles)
     **********************/
    // DFS over GraphNode.children: is 'target' reachable from 'from'?
    _gnReachable(from, target) {
        if (from === target) return true;
        const seen = new Set();
        const stack = [from];
        while (stack.length) {
            const cur = stack.pop();
            if (seen.has(cur)) continue;
            seen.add(cur);
            if (cur === target) return true;
            for (const c of cur.children) stack.push(c);
        }
        return false;
    }

    /**********************
     * Reconvergence helpers
     **********************/
    _reachableSet(start, limit = null) {
        const visited = new Set();
        const stack = [start];
        while (stack.length) {
            const cur = stack.pop();
            if (visited.has(cur)) continue;
            visited.add(cur);
            if (limit && cur === limit) continue;
            for (const c of cur.children) stack.push(c);
        }
        return visited;
    }

    _bfsDistance(start, target, limit = null) {
        if (start === target) return 0;
        const q = [{ node: start, dist: 0 }];
        const seen = new Set([start]);
        while (q.length) {
            const { node, dist } = q.shift();
            if (limit && node === limit) continue;
            for (const c of node.children) {
                if (seen.has(c)) continue;
                if (c === target) return dist + 1;
                seen.add(c);
                q.push({ node: c, dist: dist + 1 });
            }
        }
        return Infinity;
    }

    _findReconverge(children, limit = null) {
        if (!children || children.length === 0) return null;
        const sets = children.map(ch => this._reachableSet(ch, limit));
        let intersection = null;
        for (const s of sets) {
            if (intersection === null) intersection = new Set(s);
            else {
                for (const x of Array.from(intersection)) {
                    if (!s.has(x)) intersection.delete(x);
                }
            }
        }
        if (!intersection || intersection.size === 0) return null;

        let best = null;
        let bestScore = Infinity;
        for (const cand of intersection) {
            let total = 0;
            let unreachable = false;
            for (const ch of children) {
                const d = this._bfsDistance(ch, cand, limit);
                if (d === Infinity) { unreachable = true; break; }
                total += d;
            }
            if (unreachable) continue;
            if (total < bestScore) { bestScore = total; best = cand; }
        }
        return best;
    }

    /* ===================
       AST builder + simplify + render
       =================== */

    // _buildAST(node, stopNode, memo)
    // AST types:
    //  - { type: "value", val: string }
    //  - { type: "series", parts: [AST...] }
    //  - { type: "parallel", branches: [AST...] }
    _buildAST(node, stopNode = null, memo = new Map()) {
        if (!node) return null;

        // memoization per (node, stopNode)
        let stopMap = memo.get(node);
        if (!stopMap) { stopMap = new Map(); memo.set(node, stopMap); }
        if (stopMap.has(stopNode)) return stopMap.get(stopNode);

        // placeholder to prevent recursion loops
        stopMap.set(stopNode, null);

        if (stopNode && node === stopNode) {
            stopMap.set(stopNode, null);
            return null;
        }

        if (node.children.length === 0) {
            stopMap.set(stopNode, null);
            return null;
        }

        // parallel
        if (node.children.length > 1) {
            const reconverge = this._findReconverge(node.children, stopNode);

            const branches = [];
            for (let i = 0; i < node.children.length; ++i) {
                const child = node.children[i];
                // get label from branches Map using child object as key
                const branchLabel = node.branches.has(child) ? String(node.branches.get(child)) : "";

                const childAst = this._buildAST(child, reconverge ? reconverge : stopNode, memo);

                const parts = [];
                if (branchLabel !== "") parts.push({ type: "value", val: branchLabel });
                if (childAst) {
                    if (childAst.type === "series") for (const p of childAst.parts) parts.push(p);
                    else parts.push(childAst);
                }

                if (parts.length === 0) parts.push({ type: "value", val: "0" });
                const branchNode = (parts.length === 1) ? parts[0] : { type: "series", parts: parts };
                branches.push(branchNode);
            }

            const parallelNode = { type: "parallel", branches: branches };

            if (reconverge && reconverge !== stopNode) {
                const rest = this._buildAST(reconverge, stopNode, memo);
                if (rest) {
                    const out = { type: "series", parts: [parallelNode, rest] };
                    stopMap.set(stopNode, out);
                    return out;
                }
            }

            stopMap.set(stopNode, parallelNode);
            return parallelNode;
        }

        // single child -> series
        const child = node.children[0];
        const branchLabel = node.branches.has(child) ? String(node.branches.get(child)) : "";
        if (stopNode && child === stopNode) {
            if (!branchLabel || branchLabel === "") {
                stopMap.set(stopNode, null);
                return null;
            }
            const valNode = { type: "value", val: branchLabel };
            stopMap.set(stopNode, valNode);
            return valNode;
        }

        const childAst = this._buildAST(child, stopNode, memo);

        const parts = [];
        if (branchLabel !== "") parts.push({ type: "value", val: branchLabel });
        if (childAst) {
            if (childAst.type === "series") for (const p of childAst.parts) parts.push(p);
            else parts.push(childAst);
        }

        if (parts.length === 0) {
            const zero = { type: "value", val: "0" };
            stopMap.set(stopNode, zero);
            return zero;
        }

        const out = (parts.length === 1) ? parts[0] : { type: "series", parts: parts };
        stopMap.set(stopNode, out);
        return out;
    }

    _isZeroAST(ast) {
        return ast && ast.type === "value" && String(ast.val).trim() === "0";
    }

    _simplifyAST(node) {
        if (!node) return null;

        if (node.type === "value") {
            const v = (node.val === null || node.val === undefined || String(node.val).trim() === "") ? "0" : String(node.val).trim();
            return { type: "value", val: v };
        }

        if (node.type === "series") {
            const parts = [];
            for (const p of node.parts) {
                const sp = this._simplifyAST(p);
                if (!sp) continue;
                if (sp.type === "series") for (const sub of sp.parts) parts.push(sub);
                else parts.push(sp);
            }

            const nonZero = [];
            let zeros = 0;
            for (const p of parts) {
                if (this._isZeroAST(p)) zeros++;
                else nonZero.push(p);
            }

            if (nonZero.length === 0) return { type: "value", val: "0" };
            if (zeros > 0) nonZero.push({ type: "value", val: "0" });
            if (nonZero.length === 1) return nonZero[0];
            return { type: "series", parts: nonZero };
        }

        if (node.type === "parallel") {
            let branches = [];
            for (const b of node.branches) {
                const sb = this._simplifyAST(b);
                if (!sb) continue;
                if (sb.type === "parallel") for (const sub of sb.branches) branches.push(sub);
                else branches.push(sb);
            }

            if (branches.length === 0) return { type: "value", val: "0" };
            if (branches.length === 1) return branches[0];
            return { type: "parallel", branches: branches };
        }

        return null;
    }

    _astToString(node, opts = { topLevel: true }) {
        if (!node) return "";

        const render = (n) => {
            if (!n) return "";
            if (n.type === "value") return String(n.val);
            if (n.type === "series") {
                const parts = n.parts.map(p => render(p));
                return parts.join(" + ");
            }
            if (n.type === "parallel") {
                const branches = n.branches.map(b => {
                    const s = render(b);
                    if (s.indexOf(" + ") >= 0) return "(" + s + ")";
                    return s;
                });
                return "(" + branches.join(" // ") + ")";
            }
            return "";
        };

        let out = render(node);
        if (opts.topLevel) out = this._stripOuterParensText(out);
        return out;
    }

    _stripOuterParensText(s) {
        if (!s) return s;
        s = s.trim();
        while (s.length >= 2 && s[0] === '(' && s[s.length - 1] === ')') {
            let depth = 0;
            let valid = true;
            for (let i = 0; i < s.length; ++i) {
                let ch = s[i];
                if (ch === '(') depth++;
                else if (ch === ')') depth--;
                if (depth === 0 && i < s.length - 1) { valid = false; break; }
            }
            if (!valid) break;
            s = s.substring(1, s.length - 1).trim();
        }
        return s;
    }

    buildString(startNode, visited = []) {
        if (!startNode) return "";
        const rawAst = this._buildAST(startNode, null, new Map());
        const simp = this._simplifyAST(rawAst);
        return this._astToString(simp, { topLevel: true });
    }
}


// helper for debugging
function printGraphNode(node, visitedPrint = []) {
    if (visitedPrint.includes(node)) return -1;
    visitedPrint.push(node);

    let local_str = `${node.value}:`;
    let strs = [];

    for (let i = 0; i < node.children.length; ++i) {
        let child = node.children[i];
        local_str += ` ${child.value}`;

        let res = printGraphNode(child, visitedPrint);
        if (typeof res === "string") strs.push(res);
    }

    for (let s in strs) {
        local_str += `\n` + strs[s];
    }

    return local_str;
}

Testing notes

• Manually draw three 1k resistors between StartNode and DestNode on separate rails: expression should become 1k // 1k // 1k and evaluate to 333.3333333…
• Validate graphs previously showing 5:6 and 6:5 no longer create a cycle in printGraphNode.
• Validate branching case 1: 2 4 6 | 2: end | 4: 3 | 3: 2 | 6: 7 | 7: 2 keeps all three branches intact.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.