Skip to content

Add DRC error when routed trace is thinner than requested#132

Merged
seveibar merged 12 commits intotscircuit:mainfrom
mohan-bee:check-trace-match
Apr 16, 2026
Merged

Add DRC error when routed trace is thinner than requested#132
seveibar merged 12 commits intotscircuit:mainfrom
mohan-bee:check-trace-match

Conversation

@mohan-bee
Copy link
Copy Markdown
Contributor

Motivation
Right now, if the autorouter cannot honor an explicit trace thickness because of clearance limits, it quietly routes a thinner trace with no feedback. That makes routing results misleading and hard to debug. Showing an error makes the problem visible and helps users understand that their requested thickness was not actually achieved.

This change adds a routing check that reports a pcb_trace_error when a trace is routed thinner than its explicit requested thickness. Instead of silently accepting a narrower routed trace, the checker now surfaces the mismatch as a DRC-style error.

@mohan-bee mohan-bee marked this pull request as draft April 15, 2026 05:11
@mohan-bee mohan-bee marked this pull request as ready for review April 16, 2026 03:54
Comment thread lib/check-source-traces-match-pcb-trace-thickness.ts Outdated
Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

include a pcb snapshot with error display on for the test, too difficult to know if this works etc. as is

@mohan-bee
Copy link
Copy Markdown
Contributor Author

include a pcb snapshot with error display on for the test, too difficult to know if this works etc. as is

can i add a repro3 ?

@mohan-bee mohan-bee requested a review from seveibar April 16, 2026 04:13
Comment thread lib/check-source-traces-match-pcb-trace-thickness.ts
Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

very good! can we make it a warning? would like to see the coordinate

@mohan-bee
Copy link
Copy Markdown
Contributor Author

yes ! sounds logic !

@mohan-bee mohan-bee requested a review from seveibar April 16, 2026 04:55
]
}),
)
.at(0)
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 is inefficient, use a regular loop and break early, you're finding all the segments then taking .at(0) at the end

sourceTrace.display_name &&
!containsCircuitJsonId(sourceTrace.display_name)
? sourceTrace.display_name
: "trace"
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.

we're not allowing this anymore. It leaves a bad error message- you have to always have a presentable name, there should be a function for getting a display name, "trace" is never acceptable

: "trace"

warnings.push({
type: "pcb_trace_thickness_warning",
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 needs to be defined in the circuit-json specification, make sure to do a PR over there, then make sure you're constructing this warning in a typesafe way

Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

very good, just need the type for the warning

@mohan-bee mohan-bee requested a review from seveibar April 16, 2026 17:01
Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

This is good but it cant be a part of runAllRoutingChecks yet, because there’s nothing for thebuser to act on and it doesnt prohibit manufacturing

@mohan-bee
Copy link
Copy Markdown
Contributor Author

making sense

@mohan-bee mohan-bee requested a review from seveibar April 16, 2026 17:15
Comment thread tests/lib/run-all-checks.test.ts Outdated

expect(standaloneWarnings).toHaveLength(1)
expect(routingIssues).toEqual([])
})
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.

you should remove this test- it's confusing to have an exclusion test

Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

Can merge after removing that one test case

@seveibar seveibar merged commit 3c82b7d into tscircuit:main Apr 16, 2026
5 checks passed
@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.

3 participants