Require a trace endpoint's pad to be on the same layer by abdalraof-albarbar · Pull Request #166 · tscircuit/checks

abdalraof-albarbar · 2026-07-02T16:59:59Z

Closes #167

Fixes the false negative from #165. checkTracesAreContiguous matched endpoints to pads by x/y only, so a top-layer wire endpoint was accepted as connected to a bottom-layer pad. Now each isPointInPad check is gated by the endpoint's layer (SMT pad → its layer, plated hole → its copper layers). Legit via routes still pass since each endpoint lands on its own pad's layer; full suite stays green.

0hmX · 2026-07-02T17:19:26Z

not sure what i am seeing

Good call — I removed that snapshot. This is a verdict-only fix (the DRC now flags a trace endpoint whose pad is on a different layer), so the board render was byte-identical on both branches and the snapshot showed nothing to compare. The layer check is covered by the numeric test instead. Mind taking another look when you get a sec? 🙏

abdalraof-albarbar · 2026-07-02T17:26:46Z

that snapshot wasn't showing anything useful. It just rendered the board, which is identical whether the check passes or fails (the fix changes the check's result, not the geometry), so there was genuinely nothing to see in it. Removed it. The layer bug is covered by the assertion test instead — a top-layer wire endpoint on a bottom-layer pad now returns an error. @0hmX @seveibar

0hmX

the fix looks valid; i am not sure i can approve this because i am not the code owner maybe @MustafaMulla29 can take a look he last edited this file

0hmX · 2026-07-04T16:21:08Z

-            isPointInPad({ x: lastPoint.x, y: lastPoint.y }, pad),
+          pads.some(
+            (pad) =>
+              isPointInPad({ x: lastPoint.x, y: lastPoint.y }, pad) &&


i do not think you need a new function isPointInPad should also check the layer

Good call — folded the layer check into isPointInPad and removed the separate isPadOnLayer.

MustafaMulla29

you should first try to reproduce it with a snapshot test along with "shouldDrawErrors: true".

abdalraof-albarbar · 2026-07-05T06:46:52Z

@MustafaMulla29 added a snapshot test rendered with shouldDrawErrors: true — it shows the flagged endpoint (a top-layer wire landing on a bottom-layer pad), so the error is now visible in the snapshot. Also folded the layer check into isPointInPad per @0hmX.

MustafaMulla29

Repro should be merged first and then the solution

abdalraof-albarbar · 2026-07-05T09:17:06Z

@MustafaMulla29 restructured as suggested: the repro (#165) now carries the shouldDrawErrors snapshot test — on the unfixed code it renders the scenario with no error drawn — and this fix flips it so the error marker appears. Repro is ready to merge first, then this.

MustafaMulla29 · 2026-07-05T11:36:00Z


 export function isPointInPad(
-  point: { x: number; y: number },
+  point: { x: number; y: number; layer?: string },


Layer can be a separate param, point is represented only with x an y

Done — isPointInPad(point, pad, layer?) now takes layer as a separate param; point stays { x, y }. All call sites pass the endpoint's layer through.

MustafaMulla29

See comment and now just update the repro snapshot and remove this repro from pr.

checkTracesAreContiguous matched an endpoint to a pad by x/y only, so a top-layer wire endpoint counted as connected to a bottom-layer pad. Gate the match on layer: isPointInPad now takes the endpoint's layer as a separate param and rejects a pad on a different layer (smt pad -> its layer, plated hole -> its copper layers). Flips the repro tests from tscircuit#165 to passing and updates the shouldDrawErrors snapshot to show the now-flagged endpoint. Co-authored-by: abdalrouf-AAA <236259618+technologyet31-create@users.noreply.github.com>

abdalraof-albarbar · 2026-07-05T11:50:45Z

@MustafaMulla29 addressed both points:

Repro removed from this PR — rebased onto main (which now has the merged repro Add a repro test for checkTracesAreContiguous ignoring pad layer #165), so the PR no longer re-adds the test file. The only change to the test is flipping the two test.failing cases to test now that the fix makes them pass.
Repro snapshot updated — regenerated trace-endpoint-on-different-layer.snap.svg; it now renders the pcb_trace_error marker on the endpoint that lands on the wrong-layer pad.
isPointInPad — layer is now a separate param, point stays { x, y } (per your inline comment).

Diff is now just the lib fix + the snapshot update + the .failing flip. Full suite green (127 pass).

MustafaMulla29 · 2026-07-05T12:26:06Z

  point: { x: number; y: number },
  pad: PcbSmtPad | PcbPlatedHole,
+  layer?: string,
 ): boolean {


See if we have a layer type we can import from circuit-json. Otherwise looks good

MustafaMulla29 · 2026-07-05T12:26:42Z

+    const padIsOnLayer =
+      pad.type === "pcb_smtpad"
+        ? pad.layer === layer
+        : pad.layers.some((padLayer) => padLayer === layer)


Simplify this, remove ternary

abdalraof-albarbar mentioned this pull request Jul 2, 2026

checkTracesAreContiguous accepts a trace endpoint connected to a pad on a different layer #167

Open

abdalraof-albarbar force-pushed the traces-layer-fix branch from aae93b9 to ff51782 Compare July 2, 2026 17:10

0hmX requested changes Jul 2, 2026

View reviewed changes

abdalraof-albarbar force-pushed the traces-layer-fix branch from ff51782 to b9add1a Compare July 2, 2026 17:26

abdalraof-albarbar requested a review from 0hmX July 2, 2026 17:49

abdalraof-albarbar force-pushed the traces-layer-fix branch from b9add1a to adf816a Compare July 4, 2026 10:34

0hmX reviewed Jul 4, 2026

View reviewed changes

MustafaMulla29 requested changes Jul 5, 2026

View reviewed changes

abdalraof-albarbar requested a review from MustafaMulla29 July 5, 2026 06:51

MustafaMulla29 reviewed Jul 5, 2026

View reviewed changes

abdalraof-albarbar force-pushed the traces-layer-fix branch from b555e1e to 59e8651 Compare July 5, 2026 09:07

abdalraof-albarbar requested a review from MustafaMulla29 July 5, 2026 09:31

MustafaMulla29 reviewed Jul 5, 2026

View reviewed changes

MustafaMulla29 requested changes Jul 5, 2026

View reviewed changes

abdalraof-albarbar force-pushed the traces-layer-fix branch from 59e8651 to 5831e27 Compare July 5, 2026 11:49

MustafaMulla29 reviewed Jul 5, 2026

View reviewed changes

MustafaMulla29 approved these changes Jul 5, 2026

View reviewed changes

Uh oh!

Conversation

abdalraof-albarbar commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdalraof-albarbar commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0hmX left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MustafaMulla29 left a comment

Choose a reason for hiding this comment

Uh oh!

abdalraof-albarbar commented Jul 5, 2026

Uh oh!

MustafaMulla29 left a comment

Choose a reason for hiding this comment

Uh oh!

abdalraof-albarbar commented Jul 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MustafaMulla29 left a comment

Choose a reason for hiding this comment

Uh oh!

abdalraof-albarbar commented Jul 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abdalraof-albarbar commented Jul 2, 2026 •

edited

Loading

abdalraof-albarbar commented Jul 2, 2026 •

edited

Loading

0hmX left a comment •

edited

Loading