diff --git a/lib/solvers/MspConnectionPairSolver/getConnectivityMapFromInputProblem.ts b/lib/solvers/MspConnectionPairSolver/getConnectivityMapFromInputProblem.ts index d3a098c..5b735b7 100644 --- a/lib/solvers/MspConnectionPairSolver/getConnectivityMapFromInputProblem.ts +++ b/lib/solvers/MspConnectionPairSolver/getConnectivityMapFromInputProblem.ts @@ -1,23 +1,81 @@ import { ConnectivityMap } from "connectivity-map" import type { InputProblem } from "lib/types/InputProblem" +/** + * Creates connectivity maps from the input problem. + * + * CRITICAL FIX: Direct connections and net connections with the same netId + * are NO LONGER automatically merged together. This prevents traces from + * incorrectly jumping/connecting to other pins that happen to share the same + * net name but are not part of the same electrical connection. + * + * The connectivity is determined by: + * 1. Direct connections: Each is treated as its own connectivity island + * 2. Net connections: Each is treated as its own connectivity island + * + * Two pins are ONLY considered connected if they are explicitly listed in + * the SAME directConnection or SAME netConnection entry. Sharing a netId + * alone does NOT make pins connected. + * + * Edge cases handled: + * - Multiple directConnections with the same netId remain separate + * - Multiple netConnections with the same netId remain separate + * - Empty pinIds arrays are safely handled + * - Single-pin connections are valid (won't create pairs) + */ export const getConnectivityMapsFromInputProblem = ( inputProblem: InputProblem, ): { directConnMap: ConnectivityMap; netConnMap: ConnectivityMap } => { const directConnMap = new ConnectivityMap({}) - for (const directConn of inputProblem.directConnections) { - directConnMap.addConnections([ - directConn.netId - ? [directConn.netId, ...directConn.pinIds] - : directConn.pinIds, - ]) + // DEFENSIVE FIX: Each directConnection is added as its own connectivity island. + // Previously, directConnections with the same netId were merged together, + // causing traces to incorrectly connect to pins from different directConnections. + // Now we use a unique synthetic net ID (dc_) to keep them separate. + // + // Note: directConn.pinIds is typed as [PinId, PinId] tuple (always 2 elements), + // so no validation for empty arrays is needed here. + for (let i = 0; i < inputProblem.directConnections.length; i++) { + const directConn = inputProblem.directConnections[i]! + + // Create a unique synthetic net ID for this direct connection. + // This prevents different directConnections with the same netId from + // being merged, which was causing traces to jump to unrelated pins. + const syntheticNetId = `dc_${i}` + directConnMap.addConnections([[syntheticNetId, ...directConn.pinIds]]) } const netConnMap = new ConnectivityMap(directConnMap.netMap) - for (const netConn of inputProblem.netConnections) { - netConnMap.addConnections([[netConn.netId, ...netConn.pinIds]]) + // DEFENSIVE: Each netConnection is added as a separate connectivity island. + // We use a unique synthetic net ID (nc_) to prevent different + // netConnection objects with the same netId from being merged together. + for (let i = 0; i < inputProblem.netConnections.length; i++) { + const netConn = inputProblem.netConnections[i]! + + // Validate: skip if no pins to connect + if (!netConn.pinIds || netConn.pinIds.length === 0) { + continue + } + + // Check if any pins in this netConnection are already connected via + // directConnections. If so, we extend that existing connectivity. + // Otherwise, create a unique island for this netConnection. + const existingNetId = netConn.pinIds + .map((pinId) => directConnMap.getNetConnectedToId(pinId)) + .find((netId) => netId !== undefined) + + if (existingNetId) { + // At least one pin is already in the direct connection map, + // add all pins from this netConnection to that existing net + netConnMap.addConnections([[existingNetId, ...netConn.pinIds]]) + } else { + // No pins are already connected via directConnections. + // Create a unique synthetic net ID to keep this netConnection + // separate from other netConnections with the same netId. + const syntheticNetId = `nc_${i}` + netConnMap.addConnections([[syntheticNetId, ...netConn.pinIds]]) + } } return { directConnMap, netConnMap } diff --git a/tests/solvers/MspConnectionPairSolver/MspConnectionPairSolver_sameNetIdNoMerge.test.ts b/tests/solvers/MspConnectionPairSolver/MspConnectionPairSolver_sameNetIdNoMerge.test.ts new file mode 100644 index 0000000..52677a3 --- /dev/null +++ b/tests/solvers/MspConnectionPairSolver/MspConnectionPairSolver_sameNetIdNoMerge.test.ts @@ -0,0 +1,296 @@ +import { MspConnectionPairSolver } from "lib/solvers/MspConnectionPairSolver/MspConnectionPairSolver" +import { getConnectivityMapsFromInputProblem } from "lib/solvers/MspConnectionPairSolver/getConnectivityMapFromInputProblem" +import { test, expect } from "bun:test" +import type { InputProblem } from "lib/types/InputProblem" + +/** + * Test for bug fix: tscircuit/core#1498 + * + * ISSUE: Schematic traces with the same net name were incorrectly + * jumping/connecting to each other when they shouldn't. + * + * ROOT CAUSE: The getConnectivityMapsFromInputProblem function was + * merging all directConnections and netConnections with the same netId + * into a single connectivity group. + * + * FIX: Each directConnection and netConnection is now treated as its own + * connectivity island. Pins are only considered connected if they are + * explicitly listed in the SAME connection entry. + */ + +test("directConnections with same netId should NOT be merged together", () => { + // Two separate directConnections both have netId "GND" + // They should NOT be connected to each other + const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U1.1", x: -0.5, y: 0 }, + { pinId: "U1.2", x: 0.5, y: 0 }, + ], + }, + { + chipId: "U2", + center: { x: 3, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U2.1", x: 2.5, y: 0 }, + { pinId: "U2.2", x: 3.5, y: 0 }, + ], + }, + { + chipId: "C1", + center: { x: 0, y: 2 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "C1.1", x: 0, y: 2 }], + }, + { + chipId: "C2", + center: { x: 3, y: 2 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "C2.1", x: 3, y: 2 }], + }, + ], + // Both directConnections have the same netId "GND" + // but they should NOT be merged together + directConnections: [ + { pinIds: ["U1.1", "C1.1"], netId: "GND" }, + { pinIds: ["U2.1", "C2.1"], netId: "GND" }, + ], + netConnections: [], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, + } + + const { directConnMap, netConnMap } = + getConnectivityMapsFromInputProblem(inputProblem) + + // U1.1 should be connected to C1.1 (same directConnection) + expect(directConnMap.areIdsConnected("U1.1", "C1.1")).toBe(true) + + // U2.1 should be connected to C2.1 (same directConnection) + expect(directConnMap.areIdsConnected("U2.1", "C2.1")).toBe(true) + + // U1.1 should NOT be connected to U2.1 (different directConnections) + // This was the bug - they were being merged because they shared netId "GND" + expect(directConnMap.areIdsConnected("U1.1", "U2.1")).toBe(false) + + // U1.1 should NOT be connected to C2.1 (different directConnections) + expect(directConnMap.areIdsConnected("U1.1", "C2.1")).toBe(false) + + // C1.1 should NOT be connected to C2.1 (different directConnections) + expect(directConnMap.areIdsConnected("C1.1", "C2.1")).toBe(false) +}) + +test("netConnections with same netId should NOT be merged together", () => { + const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U1.1", x: -0.5, y: 0 }, + { pinId: "U1.2", x: 0.5, y: 0 }, + ], + }, + { + chipId: "U2", + center: { x: 5, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U2.1", x: 4.5, y: 0 }, + { pinId: "U2.2", x: 5.5, y: 0 }, + ], + }, + ], + directConnections: [], + // Two netConnections with the same netId "GND" + // They should NOT be merged together + netConnections: [ + { pinIds: ["U1.1", "U1.2"], netId: "GND" }, + { pinIds: ["U2.1", "U2.2"], netId: "GND" }, + ], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, + } + + const { netConnMap } = getConnectivityMapsFromInputProblem(inputProblem) + + // U1.1 should be connected to U1.2 (same netConnection) + expect(netConnMap.areIdsConnected("U1.1", "U1.2")).toBe(true) + + // U2.1 should be connected to U2.2 (same netConnection) + expect(netConnMap.areIdsConnected("U2.1", "U2.2")).toBe(true) + + // U1.1 should NOT be connected to U2.1 (different netConnections) + expect(netConnMap.areIdsConnected("U1.1", "U2.1")).toBe(false) + + // U1.2 should NOT be connected to U2.2 (different netConnections) + expect(netConnMap.areIdsConnected("U1.2", "U2.2")).toBe(false) +}) + +test("MspConnectionPairSolver should not create pairs across different directConnections with same netId", () => { + const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U1.1", x: -0.5, y: 0 }, + { pinId: "U1.2", x: 0.5, y: 0 }, + ], + }, + { + chipId: "C1", + center: { x: 0, y: 1 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "C1.1", x: 0, y: 1 }], + }, + { + chipId: "U2", + center: { x: 3, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U2.1", x: 2.5, y: 0 }, + { pinId: "U2.2", x: 3.5, y: 0 }, + ], + }, + { + chipId: "C2", + center: { x: 3, y: 1 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "C2.1", x: 3, y: 1 }], + }, + ], + directConnections: [ + { pinIds: ["U1.1", "C1.1"], netId: "GND" }, + { pinIds: ["U2.1", "C2.1"], netId: "GND" }, + ], + netConnections: [], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, + } + + const solver = new MspConnectionPairSolver({ inputProblem }) + solver.solve() + + // Should have exactly 2 pairs: U1.1-C1.1 and U2.1-C2.1 + expect(solver.mspConnectionPairs.length).toBe(2) + + // Verify each pair only contains pins from the same directConnection + for (const pair of solver.mspConnectionPairs) { + const pinIds = pair.pins.map((p) => p.pinId) + + // Each pair should be either (U1.1, C1.1) or (U2.1, C2.1) + const isValidPair = + (pinIds.includes("U1.1") && pinIds.includes("C1.1")) || + (pinIds.includes("U2.1") && pinIds.includes("C2.1")) + + expect(isValidPair).toBe(true) + + // Should NOT have cross-connection pairs + const isCrossConnectionPair = + (pinIds.includes("U1.1") && pinIds.includes("U2.1")) || + (pinIds.includes("U1.1") && pinIds.includes("C2.1")) || + (pinIds.includes("C1.1") && pinIds.includes("U2.1")) || + (pinIds.includes("C1.1") && pinIds.includes("C2.1")) + + expect(isCrossConnectionPair).toBe(false) + } +}) + +test("empty netConnection pinIds arrays should be handled safely", () => { + const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U1.1", x: 0, y: 0 }, + { pinId: "U1.2", x: 0.5, y: 0 }, + ], + }, + ], + directConnections: [{ pinIds: ["U1.1", "U1.2"], netId: "VCC" }], + netConnections: [ + { pinIds: [], netId: "NET1" }, // Empty - should be skipped safely + ], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, + } + + // Should not throw + const { directConnMap, netConnMap } = + getConnectivityMapsFromInputProblem(inputProblem) + + // Direct connection should exist + expect(directConnMap.getNetConnectedToId("U1.1")).toBeDefined() + expect(directConnMap.areIdsConnected("U1.1", "U1.2")).toBe(true) +}) + +test("netConnection extending a directConnection should work correctly", () => { + // A netConnection that includes a pin from an existing directConnection + // should extend that connection (add more pins to the same island) + const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + pins: [ + { pinId: "U1.1", x: -0.5, y: 0 }, + { pinId: "U1.2", x: 0.5, y: 0 }, + ], + }, + { + chipId: "C1", + center: { x: 0, y: 1 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "C1.1", x: 0, y: 1 }], + }, + { + chipId: "R1", + center: { x: 0, y: 2 }, + width: 0.5, + height: 0.5, + pins: [{ pinId: "R1.1", x: 0, y: 2 }], + }, + ], + directConnections: [{ pinIds: ["U1.1", "C1.1"], netId: "NET1" }], + // This netConnection extends the directConnection by adding R1.1 + netConnections: [{ pinIds: ["C1.1", "R1.1"], netId: "NET1" }], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, + } + + const { directConnMap, netConnMap } = + getConnectivityMapsFromInputProblem(inputProblem) + + // U1.1 and C1.1 are directly connected + expect(directConnMap.areIdsConnected("U1.1", "C1.1")).toBe(true) + + // In netConnMap, the netConnection extends the directConnection + // so U1.1, C1.1, and R1.1 should all be connected + expect(netConnMap.areIdsConnected("U1.1", "C1.1")).toBe(true) + expect(netConnMap.areIdsConnected("C1.1", "R1.1")).toBe(true) + expect(netConnMap.areIdsConnected("U1.1", "R1.1")).toBe(true) +})