Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 15, 2025

  • Analyze current Graph.buildPath implementation and identify issues
  • Fix cycle detection to avoid pruning valid parallel branches
  • Improve GraphNode mapping to use node identity instead of value
  • Test the changes with complex parallel circuit examples
  • Verify that the fix handles reconvergence correctly
  • Ensure no regression in existing functionality

Issues Identified:

  1. Current cycle detection uses visited.includes(node) which may prematurely cut off valid parallel paths
  2. GraphNode mapping uses startNode.value which can cause collisions when multiple nodes share the same value
  3. The buildPath method may not properly handle complex parallel-series combinations

Planned Changes:

  • Modify cycle detection to be more selective about which paths to prune
  • Use node object identity rather than value for GraphNode mapping
  • Improve handling of parallel branches and reconvergence points

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

Summary
This pull request improves Graph string building to correctly handle parallel branches and avoid cycles in the GraphNode structure. It addresses the issue where some valid parallel paths were pruned or cycles like 5 <-> 6 produced incorrect expressions, causing wrong results (e.g., 500 instead of 333.333...). Closes #2.

Key changes

  • Add dual BFS distance maps to guide edge selection when building GraphNode:
    • _distEnd: BFS distance to end node
    • _distStart: BFS distance from start node
  • Update buildPath to allow edges that either
    • strictly reduce distance to end (getting closer to end), OR
    • strictly increase distance from start (moving away from start),
      while skipping nodes that cannot reach end at all.
      This preserves legitimate parallel branches that initially go away from end but still reconverge.
  • Add a local GraphNode cycle guard: before adding edge startGraphNode -> child, skip it if startGraphNode is already reachable from child via existing children links (prevents cycles such as 5 -> 6 and 6 -> 5).
  • Keep mergeNodes and AST logic intact, including handling of label "0" (wires). We do not remove zeros from expressions; we only prevent structural cycles and over-pruning.

Files changed

  • scripts/graph.js

Implementation notes (comments in code are in English)

  • Graph now stores two distance maps (_distEnd, _distStart) computed via a shared _computeDistances(src) BFS helper.
  • toString() computes both maps once after locating start and end.
  • buildPath() uses the dual-distance rule to orient traversal and a GraphNode-level reachability check (_gnReachable) to prevent adding back-edges that would create cycles.
  • No other files or behaviors are modified. Existing UI labeling and AST simplification remain unchanged.

Validation

  • Case with three 1k branches in parallel yields 1k // 1k // 1k and evaluates to 333.3333…
  • Graphs that previously showed 5:6 and 6:5 back-edges no longer produce cycles; expressions are correct.
  • Graphs where branches initially move away from end (e.g., 1→4→3→2→end and 1→6→7→2→end) are preserved as parallel branches and not collapsed to a single path.

Commit message (English)
fix(graph): orient GraphNode with dual BFS and prevent cycles; preserve parallel branches; closes #2

  • Add _distEnd/_distStart maps and _computeDistances
  • Allow edges that reduce d(end) or increase d(start); skip unreachable
  • Add GraphNode cycle guard to avoid back-edge loops
  • Keep mergeNodes/AST logic unchanged; do not strip label "0" wires

This fixes incorrect pruning and cycle formation in GraphNode building that led to wrong results (e.g., 500 instead of 333.3333) and restores correct handling of parallel paths.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@espmaniac espmaniac closed this Sep 15, 2025
@espmaniac espmaniac deleted the copilot/fix-26829d51-112c-4c0e-80fc-3583fc425a51 branch September 15, 2025 12:29
Copilot AI requested a review from espmaniac September 15, 2025 12:35
Copilot stopped work on behalf of espmaniac due to an error September 15, 2025 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parallel circuit problem

2 participants