From 1bdac8af523762d93343d21fec26f483789c5d1f Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sat, 25 Jul 2026 20:09:54 +0900 Subject: [PATCH 1/3] test: pin different-net trace crossings per bug report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a helper that counts places where traces on different nets properly cross, and a test that pins the current count for every imported bug report. These are current values, not targets — several of these boards are open bugs. The point is that a routing or cleanup change which makes a board worse now fails a named assertion instead of disappearing into a regenerated SVG snapshot. Verified the guard bites: changing the expected count for bug-report-20260706T213649Z from 11 to 10 fails with 'Expected: 10, Received: 11'. --- tests/different-net-trace-crossings.test.ts | 74 +++++++++++++++ tests/fixtures/traceCrossings.ts | 100 ++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/different-net-trace-crossings.test.ts create mode 100644 tests/fixtures/traceCrossings.ts diff --git a/tests/different-net-trace-crossings.test.ts b/tests/different-net-trace-crossings.test.ts new file mode 100644 index 000000000..2f61a4a08 --- /dev/null +++ b/tests/different-net-trace-crossings.test.ts @@ -0,0 +1,74 @@ +import { expect, test } from "bun:test" +import fs from "node:fs" +import path from "node:path" +import { SchematicTracePipelineSolver } from "lib/solvers/SchematicTracePipelineSolver/SchematicTracePipelineSolver" +import { getTraceCrossings } from "tests/fixtures/traceCrossings" + +/** + * Different-net trace crossings, measured across every imported bug report. + * + * These are the current values, not a target — several of these boards are + * open bugs. The point is that the numbers are pinned: a routing or cleanup + * change that makes any board worse fails here instead of sliding into a + * regenerated SVG snapshot where a crossing is easy to miss. + * + * If a change legitimately improves a board, lower the number in the same PR. + */ +const EXPECTED_CROSSINGS: Record = { + "bug-report-20260706T213649Z": 11, + "bug-report-20260706T220324Z": 6, + "bug-report-20260721T221026Z": 5, + "bug-report-20260716T144856Z": 4, + "bug-report-20260717T022934Z": 3, + "bug-report-20260707T134549Z": 3, + "bug-report-20260708T055430Z": 2, + "bug-report-20260707T092615Z": 2, + "bug-report-20260707T230831Z": 2, + "bug-report-20260707T140410Z": 1, + "bug-report-20260707T020342Z": 1, + "bug-report-20260717T031704Z": 0, + "bug-report-20260708T053736Z": 0, + "bug-report-20260707T134722Z": 0, + "bug-report-20260707T141025Z": 0, + "bug-report-20260708T095725Z": 0, + "bug-report-20260707T141421Z": 0, + "bug-report-20260717T042845Z": 0, + "bug-report-20260724T175257Z": 0, +} + +const BUG_REPORTS_DIR = path.join(import.meta.dir, "bug-reports") + +const countCrossings = (board: string) => { + const inputPath = path.join(BUG_REPORTS_DIR, board, `${board}.json`) + const inputProblem = JSON.parse(fs.readFileSync(inputPath, "utf8")) + + const solver = new SchematicTracePipelineSolver(inputProblem as any) + solver.solve() + + // `netLabelNetLabelCollisionSolver` is the last pipeline step but only + // adjusts labels, so the final trace geometry comes from the cleanup pass + // feeding it. + const traces = + solver.traceCleanupSolver2?.getOutput().traces ?? + solver.traceLabelOverlapAvoidanceSolver?.getOutput().traces ?? + [] + + return getTraceCrossings(traces).length +} + +test("every pinned board still exists", () => { + const onDisk = fs + .readdirSync(BUG_REPORTS_DIR) + .filter((d) => fs.existsSync(path.join(BUG_REPORTS_DIR, d, `${d}.json`))) + .sort() + + // A newly imported bug report should be added above rather than silently + // going unmeasured. + expect(onDisk).toEqual(Object.keys(EXPECTED_CROSSINGS).sort()) +}) + +for (const [board, expected] of Object.entries(EXPECTED_CROSSINGS)) { + test(`${board} has ${expected} different-net trace crossings`, () => { + expect(countCrossings(board)).toBe(expected) + }) +} diff --git a/tests/fixtures/traceCrossings.ts b/tests/fixtures/traceCrossings.ts new file mode 100644 index 000000000..2a8042a73 --- /dev/null +++ b/tests/fixtures/traceCrossings.ts @@ -0,0 +1,100 @@ +import type { SolvedTracePath } from "lib/solvers/SchematicTraceLinesSolver/SchematicTraceLinesSolver" + +const EPS = 1e-9 + +const isHorizontal = ( + a: { x: number; y: number }, + b: { x: number; y: number }, +) => Math.abs(a.y - b.y) < EPS + +const isVertical = (a: { x: number; y: number }, b: { x: number; y: number }) => + Math.abs(a.x - b.x) < EPS + +type Segment = { + a: { x: number; y: number } + b: { x: number; y: number } + traceId: string + netId: string + segmentIndex: number +} + +/** + * True when a horizontal and a vertical segment properly cross — they share an + * interior point. Touching at an endpoint is excluded, since traces on the same + * net legitimately meet there. + */ +const properlyCrosses = (h: Segment, v: Segment) => { + const y = h.a.y + const x = v.a.x + + const withinH = + x > Math.min(h.a.x, h.b.x) + EPS && x < Math.max(h.a.x, h.b.x) - EPS + const withinV = + y > Math.min(v.a.y, v.b.y) + EPS && y < Math.max(v.a.y, v.b.y) - EPS + + return withinH && withinV +} + +/** + * Counts places where traces belonging to *different* nets cross each other. + * + * A schematic can't always avoid these, but each one is a readability cost, so + * this is useful as a regression guard: a change to the routing or cleanup + * solvers should not silently increase the count on a known board. + */ +export const getTraceCrossings = (traces: SolvedTracePath[]) => { + const segments: Segment[] = [] + + for (const trace of traces) { + for (let i = 0; i < trace.tracePath.length - 1; i++) { + segments.push({ + a: trace.tracePath[i]!, + b: trace.tracePath[i + 1]!, + traceId: trace.mspPairId, + netId: trace.globalConnNetId, + segmentIndex: i, + }) + } + } + + const crossings: Array<{ + aTraceId: string + aNetId: string + aSegmentIndex: number + bTraceId: string + bNetId: string + bSegmentIndex: number + at: { x: number; y: number } + }> = [] + + for (let i = 0; i < segments.length; i++) { + for (let k = i + 1; k < segments.length; k++) { + const s1 = segments[i]! + const s2 = segments[k]! + if (s1.netId === s2.netId) continue + + let h: Segment | null = null + let v: Segment | null = null + if (isHorizontal(s1.a, s1.b) && isVertical(s2.a, s2.b)) { + h = s1 + v = s2 + } else if (isVertical(s1.a, s1.b) && isHorizontal(s2.a, s2.b)) { + h = s2 + v = s1 + } + if (!h || !v || !properlyCrosses(h, v)) continue + + crossings.push({ + aTraceId: s1.traceId, + aNetId: s1.netId, + aSegmentIndex: s1.segmentIndex, + bTraceId: s2.traceId, + bNetId: s2.netId, + bSegmentIndex: s2.segmentIndex, + at: { x: v.a.x, y: h.a.y }, + }) + } + } + + return crossings +} From 598c5bac3cea4a9f7b6aa20fafa21014ac8a276f Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sun, 26 Jul 2026 12:47:49 +0900 Subject: [PATCH 2/3] test: update the 092615Z baseline after #727 #727 (TraceOverlapShiftSolver) fixed the RP2040 USB-C overlap and moved bug-report-20260707T092615Z from 2 different-net crossings to 3. Measured either side of that merge: ca7e80c (before) TOTAL 40, this board 2 be20aa7 (#727) TOTAL 41, this board 3 Pinned at the current value with a comment recording the change, and reported it on #727 so the trade is visible rather than absorbed. --- tests/different-net-trace-crossings.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/different-net-trace-crossings.test.ts b/tests/different-net-trace-crossings.test.ts index 2f61a4a08..ca2150374 100644 --- a/tests/different-net-trace-crossings.test.ts +++ b/tests/different-net-trace-crossings.test.ts @@ -22,7 +22,10 @@ const EXPECTED_CROSSINGS: Record = { "bug-report-20260717T022934Z": 3, "bug-report-20260707T134549Z": 3, "bug-report-20260708T055430Z": 2, - "bug-report-20260707T092615Z": 2, + // Was 2 before #727 (TraceOverlapShiftSolver); that PR fixed the RP2040 + // USB-C overlap and moved this board from 2 to 3. Pinned at the current + // value with the regression reported on #727 rather than silently accepted. + "bug-report-20260707T092615Z": 3, "bug-report-20260707T230831Z": 2, "bug-report-20260707T140410Z": 1, "bug-report-20260707T020342Z": 1, From b6bab95a82e45dfa182923a363229a8fa9ba4e4d Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sun, 26 Jul 2026 14:46:15 +0900 Subject: [PATCH 3/3] test: pin different-net overlaps alongside crossings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A crossings-only count is one-sided: two nets that cross can always be made to stop crossing by running them along each other instead, which reads as a single line and hides a connection. Counting only crossings scores that as an improvement. #727 is the concrete case. On bug-report-20260707T092615Z it moved the board from 2 crossings / 1 overlap to 3 crossings / 0 overlaps, and both numbers involve the same pair of segments (C_SENSE.2-C1.2 and available-net-orientation-13-BAT_POS) — it separated two nets drawn on top of each other across a 0.141 span and left them crossing at a point. That is a trade in the right direction, but a crossings-only guard flags it as a regression. Pinning both quantities makes the trade visible instead of scoring it. --- tests/different-net-trace-crossings.test.ts | 79 ++++++++++------- tests/fixtures/traceCrossings.ts | 94 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 31 deletions(-) diff --git a/tests/different-net-trace-crossings.test.ts b/tests/different-net-trace-crossings.test.ts index ca2150374..96bf0afb4 100644 --- a/tests/different-net-trace-crossings.test.ts +++ b/tests/different-net-trace-crossings.test.ts @@ -2,46 +2,60 @@ import { expect, test } from "bun:test" import fs from "node:fs" import path from "node:path" import { SchematicTracePipelineSolver } from "lib/solvers/SchematicTracePipelineSolver/SchematicTracePipelineSolver" -import { getTraceCrossings } from "tests/fixtures/traceCrossings" +import { + getTraceCrossings, + getTraceOverlaps, +} from "tests/fixtures/traceCrossings" /** - * Different-net trace crossings, measured across every imported bug report. + * Different-net trace crossings and collinear overlaps, measured across every + * imported bug report. * * These are the current values, not a target — several of these boards are * open bugs. The point is that the numbers are pinned: a routing or cleanup * change that makes any board worse fails here instead of sliding into a * regenerated SVG snapshot where a crossing is easy to miss. * + * Both numbers are pinned because either one alone is gameable. Two nets that + * cross can always be "fixed" by making them run along each other instead, and + * an overlap can always be "fixed" by introducing a crossing. Neither is + * strictly better, so a change that trades one for the other should be visible + * and deliberate rather than scored as an improvement by a one-sided count. + * * If a change legitimately improves a board, lower the number in the same PR. */ -const EXPECTED_CROSSINGS: Record = { - "bug-report-20260706T213649Z": 11, - "bug-report-20260706T220324Z": 6, - "bug-report-20260721T221026Z": 5, - "bug-report-20260716T144856Z": 4, - "bug-report-20260717T022934Z": 3, - "bug-report-20260707T134549Z": 3, - "bug-report-20260708T055430Z": 2, - // Was 2 before #727 (TraceOverlapShiftSolver); that PR fixed the RP2040 - // USB-C overlap and moved this board from 2 to 3. Pinned at the current - // value with the regression reported on #727 rather than silently accepted. - "bug-report-20260707T092615Z": 3, - "bug-report-20260707T230831Z": 2, - "bug-report-20260707T140410Z": 1, - "bug-report-20260707T020342Z": 1, - "bug-report-20260717T031704Z": 0, - "bug-report-20260708T053736Z": 0, - "bug-report-20260707T134722Z": 0, - "bug-report-20260707T141025Z": 0, - "bug-report-20260708T095725Z": 0, - "bug-report-20260707T141421Z": 0, - "bug-report-20260717T042845Z": 0, - "bug-report-20260724T175257Z": 0, +const EXPECTED: Record = { + "bug-report-20260706T213649Z": { crossings: 11, overlaps: 0 }, + "bug-report-20260706T220324Z": { crossings: 6, overlaps: 0 }, + "bug-report-20260721T221026Z": { crossings: 5, overlaps: 0 }, + "bug-report-20260716T144856Z": { crossings: 4, overlaps: 0 }, + "bug-report-20260717T022934Z": { crossings: 3, overlaps: 0 }, + "bug-report-20260707T134549Z": { crossings: 3, overlaps: 0 }, + "bug-report-20260708T055430Z": { crossings: 2, overlaps: 0 }, + // #727 moved this board from 2 crossings / 1 overlap to 3 crossings / 0 + // overlaps. Both numbers involve the same pair of segments + // (`C_SENSE.2-C1.2` and `available-net-orientation-13-BAT_POS`), so that + // change separated two nets that had been drawn on top of each other over a + // 0.141-length span and left them crossing at a point instead. That is a + // deliberate trade in the right direction, not a regression — which is only + // visible because both quantities are pinned here. + "bug-report-20260707T092615Z": { crossings: 3, overlaps: 0 }, + "bug-report-20260707T230831Z": { crossings: 2, overlaps: 0 }, + "bug-report-20260707T140410Z": { crossings: 1, overlaps: 0 }, + "bug-report-20260707T020342Z": { crossings: 1, overlaps: 0 }, + "bug-report-20260717T031704Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260708T053736Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260707T134722Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260707T141025Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260708T095725Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260707T141421Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260717T042845Z": { crossings: 0, overlaps: 0 }, + "bug-report-20260724T175257Z": { crossings: 0, overlaps: 0 }, } const BUG_REPORTS_DIR = path.join(import.meta.dir, "bug-reports") -const countCrossings = (board: string) => { +const measure = (board: string) => { const inputPath = path.join(BUG_REPORTS_DIR, board, `${board}.json`) const inputProblem = JSON.parse(fs.readFileSync(inputPath, "utf8")) @@ -56,7 +70,10 @@ const countCrossings = (board: string) => { solver.traceLabelOverlapAvoidanceSolver?.getOutput().traces ?? [] - return getTraceCrossings(traces).length + return { + crossings: getTraceCrossings(traces).length, + overlaps: getTraceOverlaps(traces).length, + } } test("every pinned board still exists", () => { @@ -67,11 +84,11 @@ test("every pinned board still exists", () => { // A newly imported bug report should be added above rather than silently // going unmeasured. - expect(onDisk).toEqual(Object.keys(EXPECTED_CROSSINGS).sort()) + expect(onDisk).toEqual(Object.keys(EXPECTED).sort()) }) -for (const [board, expected] of Object.entries(EXPECTED_CROSSINGS)) { - test(`${board} has ${expected} different-net trace crossings`, () => { - expect(countCrossings(board)).toBe(expected) +for (const [board, expected] of Object.entries(EXPECTED)) { + test(`${board} has ${expected.crossings} different-net trace crossings and ${expected.overlaps} overlaps`, () => { + expect(measure(board)).toEqual(expected) }) } diff --git a/tests/fixtures/traceCrossings.ts b/tests/fixtures/traceCrossings.ts index 2a8042a73..b1d64761a 100644 --- a/tests/fixtures/traceCrossings.ts +++ b/tests/fixtures/traceCrossings.ts @@ -98,3 +98,97 @@ export const getTraceCrossings = (traces: SolvedTracePath[]) => { return crossings } + +/** + * Counts places where segments of *different* nets run along each other and + * share more than a single point — a collinear overlap. + * + * This is the companion measurement to {@link getTraceCrossings}, and the two + * are needed together. A solver can always drive the crossing count down by + * letting two nets lie on top of one another instead, which reads as one line + * and hides a connection the schematic is supposed to show. Counting only + * crossings would score that as an improvement. + */ +export const getTraceOverlaps = (traces: SolvedTracePath[]) => { + const segments: Segment[] = [] + + for (const trace of traces) { + for (let i = 0; i < trace.tracePath.length - 1; i++) { + segments.push({ + a: trace.tracePath[i]!, + b: trace.tracePath[i + 1]!, + traceId: trace.mspPairId, + netId: trace.globalConnNetId, + segmentIndex: i, + }) + } + } + + const overlaps: Array<{ + aTraceId: string + aNetId: string + aSegmentIndex: number + bTraceId: string + bNetId: string + bSegmentIndex: number + axis: "horizontal" | "vertical" + length: number + }> = [] + + for (let i = 0; i < segments.length; i++) { + for (let k = i + 1; k < segments.length; k++) { + const s1 = segments[i]! + const s2 = segments[k]! + if (s1.netId === s2.netId) continue + + let axis: "horizontal" | "vertical" | null = null + let length = 0 + + if ( + isHorizontal(s1.a, s1.b) && + isHorizontal(s2.a, s2.b) && + Math.abs(s1.a.y - s2.a.y) < EPS + ) { + const low = Math.max(Math.min(s1.a.x, s1.b.x), Math.min(s2.a.x, s2.b.x)) + const high = Math.min( + Math.max(s1.a.x, s1.b.x), + Math.max(s2.a.x, s2.b.x), + ) + // A shared endpoint is a touch, not an overlap. + if (high - low > EPS) { + axis = "horizontal" + length = high - low + } + } else if ( + isVertical(s1.a, s1.b) && + isVertical(s2.a, s2.b) && + Math.abs(s1.a.x - s2.a.x) < EPS + ) { + const low = Math.max(Math.min(s1.a.y, s1.b.y), Math.min(s2.a.y, s2.b.y)) + const high = Math.min( + Math.max(s1.a.y, s1.b.y), + Math.max(s2.a.y, s2.b.y), + ) + if (high - low > EPS) { + axis = "vertical" + length = high - low + } + } + + if (!axis) continue + + overlaps.push({ + aTraceId: s1.traceId, + aNetId: s1.netId, + aSegmentIndex: s1.segmentIndex, + bTraceId: s2.traceId, + bNetId: s2.netId, + bSegmentIndex: s2.segmentIndex, + axis, + length, + }) + } + } + + return overlaps +}