Skip to content

fix(router): reconcile #672 and #677 drilled-hole legality into one API#683

Closed
ecto wants to merge 5 commits into
mainfrom
claude/musing-allen-6fee46
Closed

fix(router): reconcile #672 and #677 drilled-hole legality into one API#683
ecto wants to merge 5 commits into
mainfrom
claude/musing-allen-6fee46

Conversation

@ecto

@ecto ecto commented Jul 25, 2026

Copy link
Copy Markdown
Owner

What

Reconciles the two independent drilled-hole (hole-to-hole) legality implementations that both added to RouteSession, keeping one API.

This branch merges #672 into current main, resolves the session.rs / complete.rs conflicts so #672's richer API survives, deletes #677's drill index entirely, and rewires #677's already-merged consumers onto the survivor.

Key decisions

Survivor = #672's API. Removed DrillElement / DrillProbe / probe_drill / commit_drill / drill_elements. Kept SessionHole / probe_hole / commit_hole / probe_via_hole (SpanId-keyed, so a via barrel rips out with its copper; compaction preserved).

Ported #677's two extra behaviors onto it:

  • Grid-pitch floorroute_window_complete_pinned still floors the WinGrid pitch at drill + hole_to_hole(), so two vias one cell apart can't violate the rule.
  • ViaClass-driven barrel probebarrel_ok probes the pinned ViaClass drill via probe_hole when the caller provides one, and falls back to each net's class drill (probe_via_hole) otherwise. The layer-independent hole check is therefore never skipped (slightly broader than either PR alone).

Same-net question — narrowed the exemption to coincident barrels only. drc::check_hole_to_hole is net-agnostic, and legalize::merge_same_net_vias dedups coincident same-net drills. So #672's blanket h.net == net skip would miss a real HoleToHole: two same-net vias at distinct spots closer than hole_to_hole. The survivor now exempts only a barrel stacked on an existing same-net one (center_dist <= COINCIDENT_HOLE_EPS, matching legalize's POS_EPS); everything else — other nets and same-net drills at distinct spots — is compared net-agnostically, exactly like the DRC.

Kept #681's M7 pair coupling (strict_pair_coupling, in_pair_breakout, pair_pads, pair_gap_between) fully intact.

Reviewer notes

Testing

🤖 Generated with Claude Code

ecto and others added 5 commits July 24, 2026 21:07
The session's legality oracle only compared copper, and copper is
indexed per layer. Two vias whose layer spans do not overlap share no
copper element, so `RouteSession::probe` never compared them — while
their DRILL barrels, which are layer-independent, could physically
collide. The 2026-07-24 CM5 fab campaign found 9 such cross-net pairs
on a routed board (worst: -0.024mm overlap, /MIPI0.D3_N vs
/USB3-0.DM); they were caught only by a post-hoc scan.

Give the session a second index over drilled holes — via barrels and
through-hole pad drills — with `probe_hole` / `commit_hole`, mirroring
`drc::check_hole_to_hole` exactly: illegal when
`dist(centers) - rA - rB < rules.hole_to_hole - 1e-6`, regardless of
layer span. Hole ids share the span-id space, so rip-up removes a via's
barrel with its copper. Same-net holes stay exempt (coincident same-net
drills are the post-route merge case); net ties are not exempt, since a
tie is electrical and a drill is mechanical — exactly as the DRC treats
them.

Wired into every via-placement path: the maze's in-search `via_ok`, the
escape/stitch `via_legal`, `validate_and_commit`'s commit gate, and the
complete window router's via edges. The hole query is one small R-tree
lookup per candidate via; a 40-net CM5 subset routes byte-identically
to before at the same wall-clock.

The diff-pair repro board's rules were internally inconsistent — a
0.5mm hole-to-hole rule with 0.2mm drills on legs 0.45mm apart makes
every pair transition unmanufacturable — so it now declares the 0.2mm
rule its own geometry implies, and both transition repros assert the
twins' drills keep it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… API

Both PRs added hole-to-hole (drilled-barrel) legality to RouteSession
independently. #677 landed in main via #681 as the narrower probe_drill /
commit_drill / DrillProbe (net-agnostic, no rip-up). #672 carried the richer
SpanId-keyed SessionHole / probe_hole / commit_hole / probe_via_hole with
per-net class drills, removal + compaction, and wiring into auto/maze/pair/
complete. This merges the two, keeping #672's richer API as the survivor and
deleting #677's drill index entirely.

Ported #677's two extra behaviors onto the survivor:
- complete.rs grid-pitch floor: the WinGrid pitch is still floored at
  drill + hole_to_hole() so two vias one cell apart can never violate the rule.
- ViaClass-driven barrel probe: barrel_ok probes the exact ViaClass drill when
  the caller pins one, and falls back to each net's class drill otherwise, so
  the layer-independent hole check is never skipped.

Settled the same-net question by narrowing #672's blanket same-net exemption to
*coincident* barrels only (center distance <= COINCIDENT_HOLE_EPS). drc's
check_hole_to_hole is net-agnostic, so two same-net vias closer than the rule at
distinct spots are a real HoleToHole the blanket exemption would miss; only a
via stacked on an existing same-net one (the reuse/merge case legalization
dedups) is exempt. Pinned with hole_probe_agrees_with_drc_on_same_net_barrels,
which asserts the probe refuses exactly what check_drc flags.

Rewired the already-merged #677 consumers (cm5_verdict driver, complete.rs) from
probe_drill/commit_drill onto probe_hole/commit_hole. Kept #681's M7 pair
coupling (strict_pair_coupling, in_pair_breakout, pair_pads, pair_gap_between)
intact.

263 lib tests pass; clippy clean (one pre-existing si_claims warning unrelated).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chojiai

chojiai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Choji has used up today's reviews

Your team has reached the free plan's limit of 5 reviews per day, so Choji stepped aside on this pull request. That's a plan limit, not a reflection of the change itself.

Your free reviews reset tomorrow. To have Choji review every push without waiting, upgrade to Pro for unlimited reviews.

You're on the free plan · 5 reviews/day.

@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments
Project Deployment Actions Updated (UTC)
mecheval Ignored Ignored Jul 25, 2026 1:42pm
vcad Ignored Ignored Jul 25, 2026 1:42pm
vcad-docs Ignored Ignored Jul 25, 2026 1:42pm
vcad-mcp Ignored Ignored Jul 25, 2026 1:42pm

Request Review

@ecto

ecto commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Superseded by the direct merge of #672 into main (f851af1a, "fix(router): enforce via hole-to-hole spacing at probe time").

This PR was reconciling #672 and #677 into a single drilled-hole legality API. While it was in review, #672 landed on main already reconciled against #677 (which had merged via #681) — upstream kept #677's DrillElement/probe_drill/commit_drill naming and grafted #672's richer behaviors onto it: commit_drill now returns a SpanId for rip-up, probe_via_drill gives the net-class-drill convenience, and the auto/maze/pair wiring is in place.

Main now satisfies the reconciliation goal in full:

This PR does the identical reconciliation with hole_* naming and a coincident-only exemption. Merging it would revert a reviewed decision for no functional gain, so closing as redundant.

@ecto ecto closed this Jul 25, 2026
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.

1 participant