Skip to content

fix: correctly exclude net-label-only nets from MSP pair queue (issue #79)#140

Open
AXGZ21 wants to merge 5 commits intotscircuit:mainfrom
AXGZ21:fix/issue-79
Open

fix: correctly exclude net-label-only nets from MSP pair queue (issue #79)#140
AXGZ21 wants to merge 5 commits intotscircuit:mainfrom
AXGZ21:fix/issue-79

Conversation

@AXGZ21
Copy link

@AXGZ21 AXGZ21 commented Mar 24, 2026

Closes #79

/claim #79

Problem

When pins are connected only via netConnections (net labels), the MspConnectionPairSolver was incorrectly creating MSP pairs and drawing traces between them. Net labels are meant to represent virtual connections — they should not produce routed traces.

Root Cause

The previous fix used directConnMap.getNetConnectedToId(pinId) which performs a net-level lookup and produces false negatives for nets that mix direct connections and net labels. This caused 7 previously-passing tests to regress.

Correct Fix

Build a flat Set of all pinIds that appear in at least one directConnections entry, then exclude any globalNet whose pins are entirely absent from that set:

const directlyConnectedPins = new Set<string>()
for (const dc of inputProblem.directConnections) {
  for (const pid of dc.pinIds) {
    directlyConnectedPins.add(pid)
  }
}
this.queuedDcNetIds = Object.keys(netConnMap.netMap).filter((netId) => {
  const allIds = netConnMap.getIdsConnectedToNet(netId) as string[]
  return allIds.some((id) => directlyConnectedPins.has(id))
})

This precisely identifies "net-label-only" nets without affecting mixed nets.

Results

Metric Before After
Tests passing 42/55 50/55
Tests failing 8 0
Format check
Type check

The 5 skipped tests were already skipped on main. Snapshots updated to reflect correct output.

@vercel
Copy link

vercel bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
schematic-trace-solver Ready Ready Preview, Comment Mar 24, 2026 3:21pm

Request Review

The previous filter incorrectly used directConnMap.getNetConnectedToId()
to look up pin membership, which produced false negatives for nets that
span both directConnections and netConnections (e.g. example01's GND net).

The correct approach: build a flat set of all pinIds that appear in at
least one directConnection entry, then exclude any globalNet whose pins
are entirely absent from that set. This precisely identifies 'net-label-
only' nets — those connected solely via netConnections — without touching
nets that mix direct wires and net labels.

All 50 tests pass (was 42). Snapshots updated to reflect the corrected
output (net-label-only nets now correctly produce only annotations, not
routed traces).

Closes tscircuit#79
@AXGZ21 AXGZ21 changed the title fix: skip MSP pair creation for net-label-only connections fix: correctly exclude net-label-only nets from MSP pair queue (issue #79) Mar 24, 2026
@AXGZ21
Copy link
Author

AXGZ21 commented Mar 24, 2026

🎬 Demo: Before / After — Issue #79 (PR #140)

Recorded with — runs the solver live on both branches, captures each stage as frames, encodes 648 frames (27s) to WebM + GIF via ffmpeg.


PR #140 fix demo — before and after


What this shows

BEFORE (main) AFTER (fix/issue-79)
4 pairs queued — GND (net-label-only) incorrectly included 2 pairs — VCC + EN (direct-wire nets only)
Full pipeline traces Spurious traces drawn between net-label-only pins Clean — net labels annotated, no extra traces
bun test v1.3.11 (af24e281) 42 pass / 8 fail 50 pass / 0 fail

▶ Download full WebM (https://github.com/AXGZ21/schematic-trace-solver/blob/fix/issue-79/demo/pr140-demo.webm)

Fix

AXGZ21 added a commit to AXGZ21/schematic-trace-solver that referenced this pull request Mar 24, 2026
AXGZ21 added a commit to AXGZ21/schematic-trace-solver that referenced this pull request Mar 24, 2026
…cuit#79)

27s recorded demo showing:
- BEFORE: 4 MSP pairs queued (GND net-label bug)
- AFTER: 2 MSP pairs (VCC + EN only, correct)
- Full pipeline before/after visualizations
- Test results: 42→50 passing, 8→0 failing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix extra net label in repro61, or remove trace

1 participant