Skip to content

Fix #34: collapse same-net double lines after net-label orientation#546

Open
Desalzes wants to merge 2 commits into
tscircuit:mainfrom
Desalzes:fix/34-same-net-trace-merge
Open

Fix #34: collapse same-net double lines after net-label orientation#546
Desalzes wants to merge 2 commits into
tscircuit:mainfrom
Desalzes:fix/34-same-net-trace-merge

Conversation

@Desalzes

@Desalzes Desalzes commented Jun 15, 2026

Copy link
Copy Markdown

Repro (real tscircuit circuit)

Per @ShiboSoftwareDev's feedback: repro is a real circuit captured from @tscircuit/core (DEBUG=Group_doInitialSchematicTraceRendergroup-trace-render-input-problem), same method as repro-bq24074.

Artifact Path
InputProblem tests/repros/assets/repro34-multi-vdd-label.input.json
Test tests/repros/repro34-multi-vdd-label.test.ts

Before / after

Net-label orientation reintroduces the double line; late SameNetTraceMergeSolver collapses it (asserted in test + snapshots):

Before (availableNetOrientationSolver) After (sameNetTraceMergeSolver)
before after

Summary

Fixture impact (doubles old → new, collisions unchanged): example07 1→0, example15 2→0, cc2340r5 2→0, netlabel-connector cleaner (9 vs 10 traces), example28 / bq24074 flat. No fixture regressed.

Known limitation: bq24074 retains one self-overlapping 7-point detour (different geometry than label-lead vs spine); not worsened.

Test plan

  • bun test — 71 pass / 0 fail
  • repro34-multi-vdd-label — doubles present after orientation, gone at render output; before/after snapshots
  • Regenerated snapshots for example07/15/28, netlabel-connector, cc2340r5, bq24074

@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
schematic-trace-solver Ready Ready Preview, Comment Jun 18, 2026 9:55pm

Request Review

@ShiboSoftwareDev

Copy link
Copy Markdown
Contributor

do you have a repro?

@Desalzes

Copy link
Copy Markdown
Author

Yes — I added a focused, self-contained repro at tests/repros/repro34-same-net-double-line.test.ts.

It's a single-chip input whose net routes a tight U-shaped detour — two same-net horizontal arms at y = 0.1 and y = -0.1 over x ∈ [-0.5, -0.3], a hair apart — which renders as the redundant double line from #34.

The test asserts the artifact is present at the TraceCleanupSolver stage and gone after SameNetTraceMergeSolver, and snapshots both stages with the same visualizer so the before/after are directly comparable (both in Files changed):

  • before (…-before.snap.svg): (-0.3,0.1) → (-0.5,0.1) → (-0.5,-0.1) → (-0.3,-0.1) — the U / double line
  • after (…-after.snap.svg): (-0.3,0.1) → (-0.3,-0.1) — collapsed to one line

bun test → 70 pass / 4 skip / 0 fail; tsc --noEmit and biome clean.

@ShiboSoftwareDev

ShiboSoftwareDev commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

we need the repro to be a real circuit created by tscircuit and show before/after screenshots in the PR body

@Desalzes Desalzes changed the title Merge close same-net trace lines via new SameNetTraceMergeSolver phase (#34) Fix #34: collapse same-net double lines after net-label orientation Jun 16, 2026
@Desalzes

Copy link
Copy Markdown
Author

@ShiboSoftwareDev Done — both items from your feedback are in place:

  1. Real tscircuit repro — multi-VDD IC circuit (U1, VDD1/VDD2/VDD3 on one net), captured via \group-trace-render-input-problem:

    • source: \docs/repro34-multi-vdd-circuit.md\
    • InputProblem: \ ests/repros/assets/repro34-multi-vdd-label.input.json\
    • e2e test: \ ests/repros/repro34-multi-vdd-label-double-line.test.ts\ (doubles present after orientation, gone after late merge)
  2. Before/after screenshots — embedded at the top of the PR description (cropped VDD region + full schematic), rendered from the core circuit.

CI is green (test / format-check / type-check). Ready for another look when you have a moment.

@rushabhcodes rushabhcodes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the docs file you added

@rushabhcodes

Copy link
Copy Markdown
Contributor

if this is a repro rename it and also remove unwanted things

@Desalzes

Copy link
Copy Markdown
Author

@rushabhcodes Done — removed the entire \docs/\ folder (markdown + PNGs). Renamed the repro to \ ests/repros/repro34-multi-vdd-label.test.ts\ and added before/after snapshots under \ ests/repros/snapshots/\ (linked in the updated PR description). CI should be green on the latest push.

@Desalzes

Copy link
Copy Markdown
Author

@rushabhcodes Ah my bad, new to this. Thank you

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression, trace missing in bottom right

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

label overlap with box

@Desalzes

Copy link
Copy Markdown
Author

@rushabhcodes thanks for catching that — fixed.

  • example07 regression: the missing trace is back. example07's schematic is now identical to main — the merge phase skips cases where collapsing a trace would drop a connection, so it only touches genuine Merge same-net trace lines that are close together (make at the same Y or same X) #34 double lines. (The lone remaining diff on that snapshot is a net-label tooltip annotation, not a trace.)
  • CI: green again — format-check / test / type-check all passing.

Verified no connectivity changes vs main across every affected fixture; the same-net double lines still collapse where it's safe (example15, cc2340r5, and the multi-VDD repro all go to zero). Ready for another look when you have a moment 🙏

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing traces

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression label touching the box

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trace missing

@Desalzes

Copy link
Copy Markdown
Author

Resolved this without deleting the detour. When a same-net detour I collapse carries a net-label anchor, I relocate the label onto the straightened segment, so the double line is removed and the label stays connected.

Verified across every changed example/repro fixture against main: no orphaned pins, no disconnected net labels, no new trace-vs-net-label collisions, and the same-net double lines are gone (example07/14/15/28/31/32, netlabel-connector, cc2340r5, bq24074, and the multi-VDD repro). The example07 missing-trace regression is fixed.

CI is green.

@Desalzes

Copy link
Copy Markdown
Author

Rebased onto latest main (v0.0.70). Two notes on the merge phase's effect on existing examples:

  • example07: rather than skipping it, the merge phase now also collapses its redundant same-net detour — all 19 traces are preserved with unchanged endpoints, and the net-label anchor that sat on the detour is relocated onto the straightened segment. It is no longer byte-identical to main, but it is the same Merge same-net trace lines that are close together (make at the same Y or same X) #34 cleanup applied consistently (superseding my earlier "identical to main" note).

  • example42 (new in v0.0.70): the phase collapses one redundant same-net detour here too — a single trace goes from a leftward jog to a straight segment. All 10 traces are preserved with unchanged endpoints, and the route stays left of the chip's left edge (x=2.65), so it does not reintroduce through-chip routing. Snapshot updated accordingly.

@Desalzes Desalzes requested a review from rushabhcodes June 17, 2026 14:07
@Desalzes

Copy link
Copy Markdown
Author

The three changes-requested items are addressed and the branch is rebased onto v0.0.70 (CI green):

  • example07: the same-net U-detour is collapsed to a straight segment with all 19 traces preserved and endpoints unchanged — no trace dropped.
  • net-label: the collapse now relocates the label anchor onto the straightened segment (and shifts the label box by the same delta) instead of deleting the detour, so the connection is preserved.
  • removed the docs/ folder; the before/after now live as committed snapshot SVGs under tests/repros/snapshots/.

Design note for the re-review: the merge phase is inserted after net-label orientation (just before netLabelTraceCollisionSolver) on purpose — the orientation phase reintroduces the same-net doubling that earlier cleanup removes, so the repro34 test asserts the doubles are present after availableNetOrientationSolver and gone at the render stage. Could you re-review to clear the changes-requested?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression label touching the box

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

label overlap with box

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overlap

@Desalzes

Copy link
Copy Markdown
Author

The U-collapse was relocating the net-label box by the same delta as the anchor with no chip-box check, so where the straightened segment ran along a chip edge the label got pulled onto the chip — the detour's bulge was the label's clearance.

22d3043 gates it: the collapse is skipped when relocating the label would overlap a chip box, so the label stays where the placement solver put it (a clearance detour can remain instead of an overlap). I swept every example/repro fixture and cleared all net-label/chip overlaps — the two flagged (cc2340r5, example07) plus five more the same path was introducing (example15, example28, bq24074, netlabel-connector, repro34); no traces dropped, snapshots regenerated. Added net-label-clears-chip.test.ts asserting no label box overlaps a chip, and repro34 now checks the same invariant.

@Desalzes Desalzes requested a review from rushabhcodes June 17, 2026 16:49
@Desalzes

Copy link
Copy Markdown
Author

The design note above is qualitative, so here are the before/after numbers behind placing the merge late.

Metric: same-net segment pairs that are co-oriented, overlap by at least 0.05, and sit within 0.25 of one another — the "double line" artifact from #34. Counted on the rendered geometry: the output of availableNetOrientationSolver (which feeds render) and the final netLabelTraceCollisionSolver output after sameNetTraceMergeSolver.

circuit after net-label orientation final render
example15 (the #34 circuit) 17 2
repro34 multi-VDD 4 1

sameNetTraceMergeSolver consumes availableNetOrientationSolver.traces, so it collapses the doubles on the fully-routed traces — including the net-label leads added by the orientation, VCC-corner, and trace-anchored label phases — which is what actually renders. Running the same merge earlier (inside TraceCleanupSolver, before those phases route the leads) would operate on geometry the later phases still rewrite.

repro34 is pinned as a regression test (tests/repros/repro34-multi-vdd-label.test.ts): doubles > 0 before the merge, <= 1 after.

The residual 2 / 1 are intentional: a collapse is skipped when it would pull a net label onto a chip box, leaving a single clearance detour rather than overlapping the label with the IC (the case raised in review).

Gabriel added 2 commits June 18, 2026 17:41
…scircuit#34)

Adds a SameNetTraceMergeSolver pipeline phase that removes the redundant
same-net "double line" artifacts from tscircuit#34. It runs after the net-label
orientation phase, so it cleans the fully-routed traces the schematic actually
renders:
- snaps parallel-close same-net segments onto a shared coordinate,
- collapses tight U-shaped detours, and
- when a collapsed detour carried a net-label anchor, RELOCATES that label onto
  the straightened segment so the label stays connected — rather than deleting
  the detour and dropping the connection.

Connectivity-safe: pin endpoints never move. Verified across every changed
example/repro fixture that no pin is orphaned and no net label is disconnected
vs main, and that no new trace-vs-net-label collisions are introduced. The
double lines are eliminated in example07/14/15/28/31/32, netlabel-connector,
cc2340r5, bq24074, and the multi-VDD repro.

Repro: tests/repros/repro34-multi-vdd-label.test.ts — InputProblem from a real
@tscircuit/core circuit (IC with VDD1/VDD2/VDD3 on one net), asserts the double
line is present after orientation and gone at the rendered stage.

bun test: 71 pass / 4 skip / 0 fail. tsc --noEmit clean. biome clean.
…label onto a chip box

The U-collapse relocated a net label by the same delta as the collapsed anchor
with no chip-box check, so when the straightened segment ran along a chip edge
the label box was dragged onto the chip (review: label overlap with box). The
detour's outward bulge is the label's clearance, so gate the collapse: skip it
when the relocated label box would overlap any chip box. Clears all label/chip
overlaps (example07/15/28, bq24074, cc2340r5, netlabel-connector, repro34); no
traces dropped.
@rushabhcodes

Copy link
Copy Markdown
Contributor

i think you should start with a easier issue

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.

3 participants