fix: don't report derived connectivity findings when autorouting was skipped - #182
Open
seveibar wants to merge 1 commit into
Open
fix: don't report derived connectivity findings when autorouting was skipped#182seveibar wants to merge 1 commit into
seveibar wants to merge 1 commit into
Conversation
…skipped When core skips autorouting for a subcircuit because of placement errors, every source trace in it lacks a pcb_trace. The connectivity checks then reported one "not connected" / "missing pcb trace" finding per net, burying the single placement error that actually caused it. Scoped to the pcb_autorouting_skipped_placement_errors_* error id and matched per subcircuit_id, so ordinary routing failures still report normally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+45
to
+86
| test("reports missing traces when autorouting actually ran", async () => { | ||
| const findings = await runAllRoutingChecks(circuitJson) | ||
| const types = findings.map((f) => f.type) | ||
| expect(types).toContain("pcb_trace_missing_error") | ||
| expect(types).toContain("pcb_port_not_connected_error") | ||
| }) | ||
|
|
||
| test("suppresses derived findings when autorouting was skipped for placement errors", async () => { | ||
| const findings = await runAllRoutingChecks([ | ||
| ...circuitJson, | ||
| skippedAutoroutingError, | ||
| ]) | ||
| const types = findings.map((f) => f.type) | ||
| expect(types).not.toContain("pcb_trace_missing_error") | ||
| expect(types).not.toContain("pcb_port_not_connected_error") | ||
| }) | ||
|
|
||
| test("does not suppress findings for other subcircuits", async () => { | ||
| const findings = await runAllRoutingChecks([ | ||
| ...circuitJson, | ||
| { | ||
| ...(skippedAutoroutingError as any), | ||
| pcb_error_id: "pcb_autorouting_skipped_placement_errors_subcircuit_9", | ||
| subcircuit_id: "subcircuit_9", | ||
| } as unknown as AnyCircuitElement, | ||
| ]) | ||
| expect(findings.map((f) => f.type)).toContain("pcb_trace_missing_error") | ||
| }) | ||
|
|
||
| test("does not suppress findings for unrelated autorouting errors", async () => { | ||
| const findings = await runAllRoutingChecks([ | ||
| ...circuitJson, | ||
| { | ||
| type: "pcb_autorouting_error", | ||
| pcb_error_id: "pcb_autorouting_error_0", | ||
| error_type: "pcb_autorouting_error", | ||
| subcircuit_id: "subcircuit_0", | ||
| message: "cF ran out of iterations", | ||
| } as unknown as AnyCircuitElement, | ||
| ]) | ||
| expect(findings.map((f) => f.type)).toContain("pcb_trace_missing_error") | ||
| }) |
Contributor
There was a problem hiding this comment.
This file contains 4 test(...) calls, but the style guide requires that a *.test.ts file may have AT MOST one test(...). Please split this into multiple numbered files, e.g. skipped-autorouting-suppresses-derived-errors1.test.ts, skipped-autorouting-suppresses-derived-errors2.test.ts, skipped-autorouting-suppresses-derived-errors3.test.ts, and skipped-autorouting-suppresses-derived-errors4.test.ts, each containing exactly one test(...) call.
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Core skips autorouting for an entire subcircuit when it has placement errors, inserting
Every source trace in that subcircuit then has no
pcb_trace, socheckSourceTracesHavePcbTracesandcheckEachPcbPortConnectedToPcbTraceseach emit one finding per net. On a real board that means hundreds ofpcb_trace_missing_error/pcb_port_not_connected_errorentries pointing at nets that have nothing wrong with them, burying the one placement error that actually caused it.Minimal shape of the confusion — a board with a single component sitting outside the outline and one unrelated, perfectly routable trace:
The last two describe R1/R2, which are fine. The router simply never ran.
Fix
Skip those two checks for source traces belonging to a subcircuit whose autorouting was skipped this way.
Deliberately narrow, because suppressing findings is easy to get wrong:
pcb_autorouting_skipped_placement_errors_error id, not to autorouting errors generally — a router that ran and failed (cF ran out of iterations) still reports everythingsubcircuit_id, so a skipped subcircuit never silences its siblingsOnly the two checks that are pure consequences of "no traces exist" are touched; overlap, clearance and length checks are unaffected.
Tests
tests/lib/skipped-autorouting-suppresses-derived-errors.test.tscovers the behaviour and both guardrails:Full suite: 145 pass.
Related
--ignore-placement-drcshould unblock autorouting, not just hide it cli#4071 —--ignore-placement-drcdoesn't currently lift the gateplacementDrcChecksDisabledon subcircuits and boards props#786 —placementDrcChecksDisabledisn't accepted on<board>, which is what the skip message tells users to set🤖 Generated with Claude Code