fix(algo): accept reversed NURBS sub-spans and run lens interior search as a last resort

[andymai]

Summary

Second landing of the NURBS endpoint-trimmed convention work (#1097), from the snapClip deepened-notch dig. Two coupled fixes:

1. Reversed sub-spans on open curves (topology)

domain_with_endpoints now also accepts a REVERSED validated sub-span on a clearly open NURBS curve, returning t₀ > t₁ so start→end interpolation stays truthful. Closed curves keep the full-domain fallback: a reversed projection pair there is usually a seam-crossing forward sub-arc, and interpolating backward would trace the complement arc. Unit tests updated: reversed-on-open now trims (and evaluates back to its endpoints); a new closed-fitted-loop test pins the reversed-pair fallback.

2. Curved-lens interior search as a last resort (algo)

A cylinder/cone wall can reach classification carrying a closed curved inner wire — e.g. a pre-existing notch outline kept through the generic split path — with no precomputed interior, because only the internal-loops splitter ran the dedicated remainder search. The curved-lens flag then aborted the analytic split unconditionally ("no contained interior for curved-lens wall"). fill_images_faces now runs cylinder_cone_remainder_interior as a last resort at the consumption point; the abort remains, but only when even the dense 256×17 grid finds no contained point.

These are coupled: reversed spans re-shape the deepened-notch split so a legitimate pre-existing lens hole lands on a generic-split wall, which without fix 2 aborts the whole analytic cut.

Verification

• Deepened-notch raw repro (arena-captured operands, RAWN=1): posBad 10 → 6; both mirrored micro-edge junction chains fully resolve. The remaining 6 unpaired edges are one already-mapped root (the cone face is not split by the marched cone×plane sections — the pave-machinery-bypass row), documented in the roadmap update in this PR.
• d4 canary green; full workspace green in release (one unrelated flaky GPU-adapter-contention test, compute_mesh_lod, passes standalone).
• New/updated unit tests in edge.rs.

---

Summary by cubic

Accept reversed NURBS sub-spans on open curves and add a last-resort interior search for curved-lens walls to avoid false analytic-split aborts. This fixes deepened-notch cases and reduces unpaired edges (posBad 10 → 6).

• Bug Fixes
  • Topology: EdgeCurve::domain_with_endpoints accepts validated reversed spans on clearly open NURBS curves, returning t0 > t1; closed curves still fall back to the full domain.
  • Algo: In fill_images_faces, run cylinder_cone_remainder_interior as a last resort for cylinder/cone walls with curved inner wires but no precomputed interior; abort only if the dense grid finds no interior point.

^{Written for commit ad59fff. Summary will update on new commits.}

[github-actions]

WASM Binary Size

File	main	PR	Delta
brepkit_wasm.wasm	5.83 MB	5.83 MB	+.2 KB (+0%)

⚠️ Size increased slightly.

[Copilot]

Pull request overview

Updates core modeling behavior to better respect endpoint-trimmed NURBS conventions during edge evaluation, and improves robustness of analytic face classification for cylinder/cone “curved-lens” walls by ensuring an interior point can still be found when earlier split paths didn’t compute one.

Changes:

• Topology: EdgeCurve::domain_with_endpoints (NURBS) now accepts validated reversed sub-spans on clearly open curves (returning t0 > t1), while preserving full-domain fallback for (nearly) closed curves.
• Algo: fill_images_faces now runs cylinder_cone_remainder_interior as a last-resort interior search for curved-lens cylinder/cone walls with no precomputed interior, aborting only if the dense search still finds no contained point.
• Builder plumbing: exposes/re-exports cylinder_cone_remainder_interior for use at the consumption point.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/topology/src/edge.rs	Accept reversed validated NURBS sub-spans on open curves; update/extend unit tests for reversed-open vs reversed-closed behavior.
crates/algo/src/builder/fill_images_faces.rs	Runs remainder-interior search as a last-resort for curved-lens walls to avoid unconditional analytic aborts when interior wasn’t precomputed.
crates/algo/src/builder/face_splitter/special_cases.rs	Adjusts visibility of cylinder_cone_remainder_interior so it can be invoked from fill_images_faces.
crates/algo/src/builder/face_splitter/mod.rs	Re-exports cylinder_cone_remainder_interior with builder-scoped visibility.
.claude/skills/roadmap/SKILL.md	Updates internal roadmap notes to reflect the revised understanding and fix path.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

Two coupled fixes to the NURBS endpoint-trimmed convention work. domain_with_endpoints now accepts reversed sub-spans (t₀ > t₁) on clearly open NURBS curves, and fill_images_faces runs cylinder_cone_remainder_interior as a last resort instead of aborting whenever a curved-lens wall arrives without a precomputed interior.

• Reversed NURBS spans (edge.rs): the new curve_open guard ((p0 - p1).length() >= WELD_EPS) correctly blocks reversed acceptance on nearly-closed curves, where a backward-interpolated span would trace the complement arc. Tests cover both the new trimming path and the closed-curve fallback.
• Last-resort interior search (fill_images_faces.rs + special_cases.rs): cylinder_cone_remainder_interior is promoted to pub and re-exported as pub(in crate::builder) so fill_images_faces can call it without touching the face-splitter's internals. The abort only fires when even the dense 256×17 grid finds no contained point, which is the correct defensive position.
• No new unwrap/expect/panic! outside test modules, no float == comparisons, and no layer-boundary violations were introduced by this PR.

Confidence Score: 4/5

Safe to merge; the two changes are tightly scoped and the fallback paths (full-domain for ambiguous closed curves, mesh-fallback when the dense grid still finds nothing) are unchanged.

Both fixes are well-reasoned and backed by updated unit tests. The only observation is that WELD_EPS serves as both the on-curve projection tolerance and the open/closed curve boundary detector without a comment explaining the shared scale; a future maintainer could change one meaning unintentionally.

crates/topology/src/edge.rs lines 127-132 — the dual use of WELD_EPS is worth a clarifying comment before the constant is ever adjusted.

Important Files Changed

Filename	Overview
crates/topology/src/edge.rs	Adds reversed-sub-span acceptance in domain_with_endpoints for clearly open NURBS curves; closed curves keep full-domain fallback. Logic is sound; one constant (WELD_EPS) now serves two roles without a clarifying comment.
crates/algo/src/builder/fill_images_faces.rs	Replaces the unconditional None abort for curved-lens walls with a last-resort call to cylinder_cone_remainder_interior; aborts only when even the dense 256×17 grid finds no contained point. Change is targeted and well-commented.
crates/algo/src/builder/face_splitter/special_cases.rs	Widens cylinder_cone_remainder_interior visibility from private to pub; no logic changes. The re-export in mod.rs restricts callers to pub(in crate::builder), keeping the surface area narrow.
crates/algo/src/builder/face_splitter/mod.rs	Adds pub(in crate::builder) use special_cases::cylinder_cone_remainder_interior to expose the interior-search helper to fill_images_faces without leaking it outside the builder module.
.claude/skills/roadmap/SKILL.md	Roadmap entry updated to record the second landing, reclassify the "phantom loop" as a legitimate notch outline, and document the remaining 6 unpaired edges and their phase-FF root cause.

_{Reviews (1): Last reviewed commit: "fix(algo): accept reversed NURBS sub-spa..." | Re-trigger Greptile}

[greptile-apps]

WELD_EPS used for two distinct roles without comment

WELD_EPS = 1e-5 is introduced as the "NURBS sub-span on-curve band" distance, i.e., how far a projected endpoint may sit off the curve before being distrusted. It is then reused on line 128 to determine whether the underlying curve itself is open or nearly-closed — a conceptually different test. The two roles happen to share the same numeric scale, but the coincidence is not explained. A reader maintaining this later may tighten the on-curve band (e.g., to match vertex tolerance) without realising it also changes the open/closed classification boundary, or vice versa. A short inline comment stating why the same value is appropriate for both would prevent this drift.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!