refactor(#562): five knot-splitting spellings collapse onto two, and the "weaker" duplicate was the stronger one

[gsdali]

Closes #562.

GeomConvert_BSplineSurfaceKnotSplitting and Geom2dConvert_BSplineCurveKnotSplitting were each wrapped twice, by two families added three releases apart. The five v0.105.0 spellings are deprecated and forward to their canonical sibling; their five bridge functions are deleted.

The issue's premise did not survive measurement

The issue called the five "strictly weaker, not differently useful". That held for four of them. Curve2D.bsplineKnotSplitValues sized its buffer from the analyzer's own count, where splitIndicesAtDiscontinuities read a fixed 256 entries and the bridge returned the count it had written — indistinguishable from a curve with exactly 256 splits. Measured on a cubic with 300 interior knots at multiplicity 3 (302 splits):

call	before	now
splitIndicesAtDiscontinuities(continuity: .c1)	256 indices, last 256	302 indices, last 302
bsplineKnotSplitValues(continuity: .c1)	302	302

Forwarding onto the canonical spelling without fixing that would have regressed the deprecated one, so OCCTCurve2DSplitAtDiscontinuities now reports the true count and the Swift caller re-reads at it — the #481 contract every other member of this family already shared.

C-layer contract change: a direct bridge caller that treated the return as "how many were written" must now clamp it. OCCTBridge is not an SPM product, so no Swift package is affected (#486).

Checkbox 4: is the surface index form worth keeping?

The information, yes. The three entry points, no.

The analyzer reports knot-table indices and OCCTSurfaceKnotSplitting converted them to parameters, so the raw form was reachable only through bsplineKnotSplitValues — which constructed the analyzer three more times to get it, once per count call and once for the values. KnotSplitResult now carries uSplitIndices/vSplitIndices from the one construction that was already happening, with uSplitParams[i] == bsplineUKnot(index: uSplitIndices[i]) by construction. That also makes the deprecated shim exact, rather than a float-equality reverse lookup through the knot table.

Both deleted values functions also took no buffer capacity at all — each wrote NbSplits() entries into a buffer the caller had sized from a separate call, safe only because the analyzer is deterministic. Recorded in the bridge header so it is not reintroduced.

The index gap that let this survive three releases

OCCTBridge.h's cross-reference index named none of the three *KnotSplitting conversion classes. That index is the map used to find every call site of a class (#510), so a by-class audit of either analyzer returned nothing. It gains GeomConvert_BSplineCurveKnotSplitting, GeomConvert_BSplineSurfaceKnotSplitting, and a --- Geom2dConvert --- section that did not exist at all, censused by call site across its six classes.

Tests

Issue562Curve2DKnotSplitDuplicateTests (OCCTGeom2dTests, 4) and Issue562SurfaceKnotSplitDuplicateTests (OCCTSurfaceTests, 5). Expectations are absolute — the fixture's own knot indices and knot table — rather than agreement between the two spellings, which stopped being evidence the moment one started forwarding to the other. Issue480Curve2DKnotSplitContinuityTests.alternateSpellingsAgree was rewritten for the same reason.

Three injected defects, each failing the tests that should catch it:

injection	caught by
bridge returns the written count again	moreSplitsThanTheFirstPassBuffer, retryBoundaryIsExact (at 257 only, correctly), deprecatedPairForwards
U indices written 0-based	indicesResolveToParams, indicesAreOneBasedAndBracketing, deprecatedTrioForwards
V continuity collapsed onto U	directionsAreIndependent — and by no existing #403 or #480 coverage

Verification

• swift build clean, no deprecation warnings (every remaining call site is inside an @available(*, deprecated) test function).
• Full swift test: 5052 tests. The 23 failures are all in Issue522ApproxC0CollapseTests/Issue491SurfaceApproxParityTests — surface approximation, untouched here. Confirmed identical (same 23 issues, same 5 test cases) against unmodified refactor/381-pass1b in a separate worktree; they clear when chore(#512) + fix(#518, #555, #522): rebuild the kernel, redesign 0016 after upstream review, add 0018 and 0019 #519's kernel binary lands.
• check-bridge-index.py 0 stale across 692 symbols / 377 classes, --self-test 5/5; count-operations.py 4299 ✓ (deprecated methods still count as entry points); check-null-handle-guards.py clean.

Docs updated in the same commit: docs/reference/Document-Geometry-Constructors.md (two duplicated sections collapsed into one deprecation pointer with a migration table), Surface-Advanced.md, Curve2D.md, API_REFERENCE.md, CHANGELOG.md.

🤖 Generated with Claude Code

[gsdali]

Review: #589 — refactor(#562): five knot-splitting spellings collapse onto two, and the "weaker" duplicate was the stronger one

Overview

GeomConvert_BSplineSurfaceKnotSplitting and Geom2dConvert_BSplineCurveKnotSplitting were each wrapped twice — once by the canonical Surface.knotSplitting/Curve2D.splitIndicesAtDiscontinuities, and again by a v0.105.0 family of five methods added three releases later. The five are deprecated and forward to their canonical sibling; the five duplicate bridge functions are deleted. Along the way, the PR finds and fixes a real defect in the canonical side (Curve2D.splitIndicesAtDiscontinuities silently truncated at 256 entries) that would have made forwarding onto it a regression for the deprecated pair — and recovers the one thing the deleted family carried that the canonical calls didn't (raw knot-table indices), rather than losing that information.

Correctness — verified against actual OCCT headers, not the PR's account of it

• The Splitting(array) → SplitValue(i) substitution is exactly equivalent, not just similar. Checked Geom2dConvert_BSplineCurveKnotSplitting.hxx and GeomConvert_BSplineSurfaceKnotSplitting.hxx directly: SplitValue(int)/USplitValue(int)/VSplitValue(int) are documented as returning the same per-index value that Splitting(array)'s bulk fill writes into that slot ("the split knot of index Index... an index in the knots table"). Switching from bulk-fill-then-copy to a per-index accessor is what makes the shared occtWriteKnotSplits template usable here (bounded write, true count reported regardless of truncation) without first allocating a full-sized array — the old code's TColStd_Array1OfInteger indices(1, splitter.NbSplits()) was exactly the over-allocation this avoids.
• The truncation bug the PR fixes in OCCTCurve2DSplitAtDiscontinuities is real and precisely characterized. The removed code computed n = std::min(NbSplits(), max) and returned n — the written count — making a genuinely-256-split curve indistinguishable from a 300-split curve truncated to 256. This is the identical defect class already fixed for the sibling functions in LawFunction.knotSplitting truncates at 100 while both its siblings retry at the true count #481 (LawFunction.knotSplitting) and Shape.pointEdgeExtrema reports an extremum as the closest point, and no point at all when the closest one is an edge end #580's neighbors — the PR correctly identifies this as "the same LawFunction.knotSplitting truncates at 100 while both its siblings retry at the true count #481 contract" rather than inventing a new pattern.
• The forwarding is behavior-preserving where it needs to be, and correctly not where it can't be. Curve2D.bsplineKnotSplitValues used to return [] for a non-BSpline curve; the new forward (splitIndicesAtDiscontinuities(...)?.map(Int32.init) ?? []) preserves that via nil-coalescing rather than propagating nil. KnotSplitResult.uSplitIndices[i] == UKnot-resolved(uSplitParams[i]) is enforced by construction (both come from the same single analyzer instantiation now), which is a stronger guarantee than the deleted family's own three-separate-construction version ever had — verified directly in the new indicesResolveToParams test rather than assumed.
• The OCCTSurfaceKnotSplitting signature change is a real C-layer break, correctly flagged as one. outUIndices/outVIndices are new required-shape parameters inserted between the existing params and their max — every direct caller of this C function needs updating, not just Swift ones. Since OCCTBridge isn't an SPM product (established precedent from Surface/Curve3D/Curve2D batch-evaluation bridge functions: three unconsolidated implementation generations per type, and OCCTGridEvalSurfaceD0/D1 use the opposite U/V-major layout from OCCTSurfaceEvaluateGrid #486), this is inert for external consumers, and the PR says so rather than glossing over it.

Test coverage

• The regression test (moreSplitsThanTheFirstPassBuffer, 302 splits against a 256-entry first pass) is discriminating in the way this project's established convention requires: last == 302 specifically distinguishes a real retry from a stale truncated-but-plausible read (last == count would trivially hold at 256 too).
• retryBoundaryIsExact checks 255/256/257 — exactly the boundary an off-by-one in the retry condition (n > 256 vs n >= 256) would get wrong.
• directionsAreIndependent is called out as covered by no pre-existing Three unaligned BSpline knot-splitting wrappers: Curve3D returns parameters, Surface returns only counts, LawFunction returns indices #403 or Surface/LawFunction knot splitting kept the continuity cap of 2 that #398 proved is a no-op #480 test — a real, previously-untested gap (U/V asked different continuities) that only exists as a question because the deleted trio couldn't ask it, and the PR explicitly checks it now that the canonical call is the only path.
• All three claimed fault injections (written-count-instead-of-true-count, 0-based indices, V collapsed onto U) map to specific tests in the PR body's table, matching this project's "prove the test catches it" bar rather than a generic pass/fail claim.
• Deprecated-spelling tests correctly carry @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") — consistent with this project's established pattern for testing a method it also marks unusable-without-warning elsewhere.

Documentation

• The new --- Geom2dConvert --- section in OCCTBridge.h's cross-reference index is itself framed as part of the root-cause fix, not just bookkeeping: the PR's own diagnosis is that this section's absence is half of why a by-class audit of either analyzer (the mechanism OCCTBridge.h cross-reference index: 139 of 418 symbol references name symbols that do not exist #510 built to catch exactly this) returned nothing for three releases. Worth noting for future audits: this closes one specific gap, not the general class — worth checking whether other OCCT classes are similarly missing from the index (out of scope here, reasonably).
• docs/reference/Document-Geometry-Constructors.md's deprecated-methods page collapses two duplicated sections into one deprecation-pointer table rather than leaving two stale API descriptions to rot — the kind of doc cleanup that's easy to skip in a mechanical deprecation and wasn't skipped here.
• The bridge header's new comment recording that the deleted family's values functions took no buffer capacity at all (relying on the analyzer's determinism for safety) is a good practice: documenting a removed hazard so it isn't reintroduced by a future "let's just add this back" without re-deriving why it was risky.

Risk / merge notes

• Pure Swift/bridge change — no kernel patch, no OCCT.xcframework rebuild, so no kernel-integration.yml involvement.
• count-operations.py unchanged at 4299 per the PR's claim (deprecated methods still count as entry points, consistent with this project's established counting rule) — worth a quick re-verification at merge time rather than trusted blind, per this project's own repeated history of PRs claiming "no count change" incorrectly, but the claim here is plausible (5 methods deleted from the bridge, but their Swift-level forwarding wrappers stay, net zero at the Swift API surface).
• The 23 pre-existing Issue522ApproxC0CollapseTests/Issue491SurfaceApproxParityTests failures the PR reports are the same known kernel-pin-gap signature seen throughout this initiative — correctly identified as unrelated and confirmed against an unmodified base in a separate worktree.
• No apparent file-overlap risk against the other currently open PRs (#590/#591 cover healing conversions and plate approximation — disjoint from the GeomConvert/Geom2dConvert knot-splitting surface this PR touches).

Verdict

Approve. The core technical substitution (Splitting(array) → per-index SplitValue) is verified exact against the actual OCCT headers, not assumed from the PR's description. The regression this PR fixes in the canonical Curve2D.splitIndicesAtDiscontinuities (256-entry silent truncation) is real, correctly diagnosed as sharing #481's already-established defect class, and would have been a silent regression for the deprecated spelling if left unfixed while forwarding. Test coverage is genuinely discriminating, including one case (directionsAreIndependent) explicitly identified as newly-covered rather than redundant with existing suites. No blocking correctness, convention, or coverage issues.