feat(#1393)!: retire Shape.splitDrafts, and carry no patch for it - #1657
Merged
Merged
Conversation
LocOpe_SplitDrafts cannot work on OCCT 8.0.1: it accepts only a planar face and then pipes along the intersection of two planes, always an infinite line, which GeomConvert refuses by documented design. Every valid call threw. The deciding fact is not the defect. Upstream deleted the class outright on 2026-08-07 in OCCT#1442, a dead-header cleanup: no caller anywhere in the OCCT tree, not even a DRAW command, which is how a total defect survived to 8.0.1 unnoticed. There was no upstream PR to open and nothing to fix for anyone else's benefit, and a carried patch would have been this project reviving a class its own maintainer had removed, with a guaranteed expiry at the first kernel bump past that commit. okf/policies/scope-boundary.md says stay faithful to OCCT. Wrapping what OCCT has deleted is the opposite of that. Removed: the Swift wrapper, the bridge function, its declaration and index row, eleven now-unused includes, the reference-page section, the API_REFERENCE entry and the refusal test. The reproducer stays: it is why the decision could be made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upstream PR was impossible: the class it fixes no longer exists on OCCT master. The GTest written for it is kept anyway, because it is the only executable statement of what a working LocOpe_SplitDrafts produces, proven against both a patched and an unpatched kernel. Patch 0034 itself is not carried and exists only in the closed PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| @@ -2035,37 +2035,6 @@ extension Shape { | |||
|
|
|||
| // MARK: - LocOpe_SplitDrafts | |||
Contributor
There was a problem hiding this comment.
WARNING: Orphaned MARK comment left after function removal
The // MARK: - LocOpe_SplitDrafts comment at line 2036 remains but the splitDrafts function it was marking has been completely removed. This orphaned marker should be deleted.
Suggested change
| // MARK: - LocOpe_SplitDrafts |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (24 files)
Previous Review Summary (commit a418212)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a418212)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (24 files)
Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 236.4K · Output: 5.8K · Cached: 561.6K |
Kilo review. The // MARK: - LocOpe_SplitDrafts heading outlived its only member and sat over an unrelated History class, which is worse than no heading: it labels the wrong thing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Fixed in |
This was referenced Sep 7, 2026
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.
What & why
Shape.splitDraftsis removed, and patch0034is not carried. Both halves of that are the samedecision.
The operation cannot work on OCCT 8.0.1.
LocOpe_SplitDrafts::Performaccepts only a planarface, then pipes along the intersection of two planes, which is always an infinite
Geom_Line, andGeomConvert::CurveToBSplineCurve's untrimmed branch refuses an infinite curve by documenteddesign (
GeomConvert.hxx: "Raises DomainError if the curve C is infinite"). Every valid callthrew. Measured in
Scripts/repro/1393-splitdrafts/.The deciding fact is not the defect. Upstream deleted the class outright on 2026-08-07 in
OCCT#1442, a dead-header cleanup:
git grep SplitDrafts upstream/masterreturns nothing, and it had no caller anywhere in the OCCTtree, not even a DRAW command, which is how a total defect survived to 8.0.1 unnoticed.
So there was no upstream PR to open, nothing to fix for anyone else's benefit, and a carried patch
would have been this project reviving a class its own maintainer had removed, with a guaranteed
expiry at the first kernel bump past that commit.
okf/policies/scope-boundary.mdsays stay faithful to OCCT.Wrapping what OCCT has deleted is the opposite of that.
Closes #1393
CHANGELOG entry
Shape.splitDraftsis removed (#1393)The operation could not succeed on any input.
LocOpe_SplitDraftsaccepts only a planar face andthen pipes along the intersection of two planes, always an infinite line, which
GeomConvert::CurveToBSplineCurverefuses by documented design, so every valid call threw and thebridge returned
nil.It is removed rather than repaired because OCCT removed it first: the class was deleted upstream on
2026-08-07 in OCCT#1442 as dead code with no
caller in the OCCT tree. Carrying a kernel patch to revive it would have expired at the next repin
and could never have been filed upstream.
Gone with it:
OCCTLocOpeSplitDrafts, its declaration and cross-reference index row, eleven unused#includes, the reference-page section and the API_REFERENCE entry. The investigation is kept inScripts/repro/1393-splitdrafts/, including the GTest written for the upstream PR that could notbe filed, since it is the only executable statement of what a working
LocOpe_SplitDraftsproduces.
SemVer impact
MAJOR.
Shape.splitDrafts(faceIndex:wire:direction:planeOrigin:planeNormal:angle:)is removed, andso is the C bridge's
OCCTLocOpeSplitDrafts. Migration: there is none, and none is needed. Thefunction returned
nilfor every input on every shipped version, so no working call site can exist.A caller that compiled against it was calling something that never did anything.
Checklist
(
Issue1393SplitDraftsTests, which asserted the refusal) is removed with the API. Nothing isleft untested, because nothing is left.
--self-testcase was run once with its subject broken: no newtests.
docs/CHANGELOG.mdis not in this diff.docs/SEMVER.mdis not in this diff.Notes for the reviewer
Verification: all ten static gates clean, including
check-bridge-index(the cross-referenceindex row is removed with the symbol) and
count-operations(4,366 down to 4,365, re-derived acrossREADME,
docs/API_REFERENCE.mdanddocs/index.md).swift buildclean,format-bridge.sh --checkclean under the pinned clang-format 22.1.8, full suite green.What is kept, deliberately.
Scripts/repro/1393-splitdrafts/stays, with an "Outcome" sectionexplaining the removal, and it gains
upstream/LocOpe_SplitDrafts_Test.cxx, the GTest from theclosed PR #1627. That file is the only executable statement of what the operation should produce,
proven both ways against patched and unpatched kernels:
IsDone=1, seven faces, one tilted byexactly the requested draft angle, and a 10-unit cube's volume rising from 1000 to 1022.04 against
the wedge's analytic 22.04. If the class is ever restored, that is the head start; if not, it is the
record of what was measured before the wrapper went.
CLAUDE.md's Known OCCT Bugs list loses a line: the standing instruction to always wrapLocOpe_SplitDrafts::Perform()intry/catchdescribes a call the bridge no longer makes. Therow in
okf/references/known-occt-bugs.mdstays and now records the disposition, because the defectwas real and the reasoning is worth keeping.