Skip to content

Check PCB trace maximum lengths during routing DRC#174

Merged
seveibar merged 2 commits into
mainfrom
agent/check-crystal-trace-lengths
Jul 24, 2026
Merged

Check PCB trace maximum lengths during routing DRC#174
seveibar merged 2 commits into
mainfrom
agent/check-crystal-trace-lengths

Conversation

@seveibar

@seveibar seveibar commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a generic routing check that compares each pcb_trace length with its source trace's max_length
  • emit pcb_trace_too_long_warning through runAllRoutingChecks
  • use a routed trace's recorded trace_length when available and compute it from route points otherwise
  • ignore absent or null maximum lengths

Architecture

This check deliberately has no crystal-specific logic. Core is responsible for teeing up Circuit JSON by assigning component-derived limits to source_trace.max_length; the normal routing DRC pipeline is responsible for producing warnings.

Companion: tscircuit/core#2781

Validation

  • bun test (132 passing)
  • bunx tsc --noEmit
  • bun run build
  • Biome and git diff --check pass

@seveibar seveibar changed the title Check crystal trace lengths across routed nets [Superseded] Check crystal trace lengths across routed nets Jul 23, 2026
@seveibar seveibar closed this Jul 23, 2026
@seveibar seveibar changed the title [Superseded] Check crystal trace lengths across routed nets Check PCB trace maximum lengths during routing DRC Jul 24, 2026
@seveibar seveibar reopened this Jul 24, 2026
@seveibar
seveibar marked this pull request as ready for review July 24, 2026 18:48
@seveibar
seveibar merged commit 39af16a into main Jul 24, 2026
5 checks passed
Comment on lines +54 to +78
test("warns when a PCB trace exceeds its source trace maximum length", () => {
expect(checkPcbTraceLengths(circuitJson)).toEqual([
{
type: "pcb_trace_too_long_warning",
pcb_trace_too_long_warning_id:
"pcb_trace_too_long_warning_overlength_pcb_trace",
warning_type: "pcb_trace_too_long_warning",
message: "PCB trace is 12.00mm long, exceeding the 10mm maximum",
pcb_trace_id: "overlength_pcb_trace",
source_trace_id: "overlength_source_trace",
source_net_id: "signal_net",
actual_trace_length: 12,
maximum_trace_length: 10,
subcircuit_id: "board",
},
])
})

test("is included in the routing check pipeline", async () => {
const results = await runAllRoutingChecks(circuitJson)

expect(
results.filter((result) => result.type === "pcb_trace_too_long_warning"),
).toEqual(checkPcbTraceLengths(circuitJson))
})

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.

This file contains two test(...) calls (lines 54 and 72), which violates the rule that a *.test.ts file may have AT MOST one test(...). Please split this into two separate numbered files, e.g. check-pcb-trace-lengths1.test.ts (for the 'warns when a PCB trace exceeds its source trace maximum length' test) and check-pcb-trace-lengths2.test.ts (for the 'is included in the routing check pipeline' test).

Spotted by Graphite (based on custom rule: Custom rule)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +29 to +32
if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
continue
}

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.

Critical bug: The continue statement skips calculating the distance from the via to the next route point. This causes traces with vias to report shorter lengths than actual, potentially missing length violations.

Impact: Traces containing vias will have their lengths underreported by the distance between the via and the next route point, causing the check to miss traces that actually exceed their maximum length.

Fix: Remove the continue statement and let the code fall through to calculate the distance to the next point:

if (routePoint.route_type === "via") {
  traceLength += DEFAULT_VIA_LENGTH_MM
  // Don't continue - still need to calculate distance to next point
}
Suggested change
if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
continue
}
if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
// Don't continue - still need to calculate distance to next point
}

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@tscircuitbot

Copy link
Copy Markdown
Contributor

Thank you for your contribution! 🎉

PR Rating: ⭐⭐
Impact: Minor

Track your contributions and see the leaderboard at: tscircuit Contribution Tracker


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.

2 participants