Skip to content

docs(#532): record the upstream filing for patch 0020 - #775

Merged
gsdali merged 1 commit into
refactor/381-pass1bfrom
docs/532-upstream-occt-pr-1447
Aug 7, 2026
Merged

docs(#532): record the upstream filing for patch 0020#775
gsdali merged 1 commit into
refactor/381-pass1bfrom
docs/532-upstream-occt-pr-1447

Conversation

@gsdali

@gsdali gsdali commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

What & why

Housekeeping for #532: patch 0020 (BRepFeat_MakeCylindricalHole tool part selection) has been
carried since PR #587 but was never filed upstream. #657's audit (PR #737) confirmed it was ready
to send and drafted the text; this PR does the actual filing and records the result.

Re-verified the defect directly against current upstream master, not just the V8_0_0_p1
baseline the original measurement used: read all four affected call sites and the two correct
callers (BRepFeat_Form, BRepFeat_RibSlot) on master, then compiled
BRepFeat_MakeCylindricalHole.cxx from master as an override translation unit, once unmodified
and once with patch 0020 applied, and ran the existing
Scripts/repro/532-cylindrical-hole-part-selection/
probe against both, linked ahead of the pinned OCCT 8.0.1 archive. The unmodified build reproduces
every figure the existing writeup records as "before", including the oversized-radius
InvalidPlacement status; the patched build reproduces every "after" figure. The touched file is
byte-identical between master and our pin, so this is the same change on both.

Filed as Open-Cascade-SAS/OCCT#1447: a fix PR
with no companion repro issue, per
okf/policies/upstream-occt-style.md and the precedent of
0018, 0019, 0021 and 0024 (an OCCT maintainer's own comment on
OCCT#1409 asked
contributors not to open a separate issue when a fix is already in hand).

This PR touches only CLAUDE.md and Scripts/patches/README.md. No source, test, or public API
change; docs/CHANGELOG.md and docs/SEMVER.md are untouched per
okf/policies/changelog-on-merge.md and
okf/policies/semver-at-release.md (entries below instead).

Not closing #532: the issue itself was closed by PR #587 when the fix was carried; this is a
follow-up on the separate, still-open upstream-filing housekeeping in #657.

CHANGELOG entry

Carried patch 0020 (#532, BRepFeat_MakeCylindricalHole tool part selection) is now filed upstream as OCCT#1447

Re-verified the defect directly against current upstream master before filing: the four affected
call sites, the two correct callers in the same builder, and the second, unfixed defect in the same
heuristic are all unchanged from the V8_0_0_p1-based measurement this patch was originally built
against, and the touched file is byte-identical between master and our pin. Confirmed by compiling
BRepFeat_MakeCylindricalHole.cxx from master as an override translation unit, once unmodified
and once patched, and running the existing reproducer against both: every "before" and "after"
figure in the original writeup reproduces exactly, including the oversized-radius status change.
Filed as Open-Cascade-SAS/OCCT#1447, PR only
per okf/policies/upstream-occt-style.md. No change to OCCTSwift's own behavior or public API.

SemVer impact

NONE. Documentation only (CLAUDE.md, Scripts/patches/README.md); no source, test, or public API
change. The upstream PR proposes a change to OCCT itself, not to this package.

Checklist

  • This is a documentation-only change; no new production code path exists to unit-test.
  • N/A: no new test or --self-test case in this PR.
  • The CHANGELOG entry above is complete, and docs/CHANGELOG.md is not in this diff.
  • The SemVer impact above is stated, and docs/SEMVER.md is not in this diff.

Notes for the reviewer

  • All five gate scripts and their --self-tests pass locally (check-bridge-index 18/18,
    check-null-handle-guards 24/24, check-docs-defaults 13/13, derive-bridge-header-split --verify 8/8, count-operations consistent at 4306), as expected for a docs-only change.
  • The upstream PR title was edited once after opening, to name what it replaces ("not the cut
    result") rather than only what it does, and its body gained a short footnote explaining the
    PerformBlind(20) figure (1178.0972, measured from the axis origin rather than the plate's
    entry face), matching a clarification already present in Ten upstream OCCT PRs now sit on a pre-8.0.1 base, and patch 0020 was never filed at all #657's drafted text.
  • Not merging this PR; opened for review only, per the task that produced it.

Filed as Open-Cascade-SAS/OCCT#1447. PR only, no companion issue, per
okf/policies/upstream-occt-style.md and the precedent of 0018, 0019, 0021 and
0024: the fix was ready, so the PR description carries the repro and root
cause a standalone issue would have.

Re-verified the defect directly against current upstream master before
filing, rather than trusting the V8_0_0_p1-based measurement in the existing
writeup: read all four call sites and the two correct callers on master, then
compiled BRepFeat_MakeCylindricalHole.cxx from master as an override
translation unit, once unmodified and once with patch 0020 applied, and ran
the existing Scripts/repro/532-cylindrical-hole-part-selection/ probe against
both, linked ahead of the pinned OCCT 8.0.1 archive. The unmodified build
reproduces every figure the original writeup reports as "before", including
the oversized-radius InvalidPlacement status; the patched build reproduces
every "after" figure. The touched file is byte-identical between master and
our pin.
@gsdali

gsdali commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

The deviation was correct and my dispatch instruction was wrong. Recording that here so the reasoning
is not re-litigated.

I asked for an issue and a PR, "following the repro-then-fix pairing our other filings use", while in
the same prompt telling the agent to read okf/policies/ for upstream contribution style. Those
contradict each other. okf/policies/upstream-occt-style.md says go straight to a PR, quoting OCCT
maintainer dpasukhi on OCCT#1409:

Thank you for your contribution. In case if you preparing PR, no needs to create Issue.

My sentence described the practice that policy exists to retire. Verified independently: the policy
says it, dpasukhi said it, and OCCT#1420 is already PR-only, filed the day after that comment.
Confirmed by the user: PR-only is the policy.

The right outcome, and worth naming because the default is the other way. An instruction in a prompt
reads as more specific and more current than a file, so the usual failure is silent compliance, which
here would have put an unwanted issue on a third-party repo. I have recorded the general rule for
myself: point a dispatched agent at the policy, never paraphrase the process it owns, because a
restated rule is a copy with no update path.

The re-verification is the strong part

Not a text diff against master, which would only have shown the file unchanged. The agent fetched
BRepFeat_MakeCylindricalHole.cxx from current master, compiled it as an override translation
unit, linked it ahead of the pinned archive, and ran Scripts/repro/532-cylindrical-hole-part-selection/
against it. Unpatched reproduced every "before" figure exactly, including the oversized-radius
InvalidPlacement; patched reproduced every "after" figure exactly.

So "still live on master" is measured against a running binary rather than inferred from a hash, which
matters because the whole point of the check was that upstream might have fixed it while we were not
looking.

OCCT#1447 is 4 lines across one file, four
SetOperation(Fuse, true) calls with a one-line comment each, matching the concise style the same
policy requires. That closes the last carried patch with no upstream presence: all fifteen are now
either shipped, filed, or under review.

@gsdali
gsdali merged commit de780db into refactor/381-pass1b Aug 7, 2026
5 checks passed
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