fix(#633): blendedEdges reports which duplicate entries were overwritten

[gsdali]

What & why

BRepFilletAPI_MakeFillet::Add(radius, edge) resolves an edge's own slot within its fillet
contour and writes there, so blendedEdges(_:) naming the same edge index twice in its
edgeRadii list writes that slot twice: the second Add silently overwrites the first radius,
with no exception, no false return, and nothing in the result to say a radius was ever discarded.

Re-measured on this tree rather than trusted from the issue (swift run Censuses cluster-b):
blendedEdges([(0, 2.0), (0, 5.0)]) gives the identical volume (946.349541) to
blendedEdges([(0, 5.0)]) and differs from blendedEdges([(0, 2.0)]) (991.415927) -- last-wins,
confirmed.

Closes #633

This is the last open member of Cluster B (#665).

The decision

Sequenced deliberately after #639, which decided the reporting mechanism for this same family's
other silent axis (an edge OCCT declines outright) and, in its own PR, recommended extending the
same Shape.FilletResult shape to also cover this issue's axis rather than re-deriving a
mechanism. That recommendation is taken here; I found no reason to deviate from it.

Report, do not reject or pick a "winning" direction. Converging blendedEdges(_:) onto reject,
or onto first-wins (matching the chamfer family, measured below), would change what it returns for
every existing caller who happens to name an edge twice and currently gets a built shape -- a
behaviour change wider than this issue, and the same move #639 rejected for its own axis.
blendedEdges(_:) itself is byte-for-byte unchanged.

Fillet and chamfer disagree on duplicate-index direction, measured again rather than assumed.
Scripts/repro/cluster-b-fillet-edge-contract/'s own headline finding stands: the fillet family
(blendedEdges, filletEvolving, FilletBuilder.addEdge) is last-wins, the chamfer family
(chamferedTwoDistances, chamferedDistAngle, ChamferBuilder.addEdge) is first-wins, both
internally consistent within their own family. Unifying the two onto one direction is a bigger,
separate decision (picking a direction and changing it for a whole family) than this issue's own
title suggests, and is left open.

What changed

blendedEdgesWithReport(_:), a new WithReport sibling, returns the existing
Shape.FilletResult -- extended with a second field, overwrittenDuplicateIndices -- rather than
a new result type (#490's standing lesson against parallel encodings of one idea). It mirrors
declinedEdgeIndices's own convention: an edge requested three times, with two of those overwritten,
reports [edge, edge], not [edge] -- the count matches how many entries of the caller's list were
discarded, not how many distinct edges were involved.

Computing the duplicate report needs no OCCT round trip at all: an edge index maps to exactly one
edge via Shape's own TopExp enumeration, so which entries lose is a property of the caller's
edgeRadii array alone, resolved Swift-side (Shape.overwrittenDuplicateIndices(in:),
Shape+Modeling.swift).

blendedEdgesWithReport(_:) also adopts #639's declined-edge mechanism for this entry point --
OCCTShapeBlendEdges gained the same two nullable trailing out-parameters
OCCTShapeFilletEdges/OCCTShapeFilletEdgesLinear already carry, threaded through the shared
occtShapeFilletEdgeList skeleton -- rather than leaving that field permanently empty for this one
sibling. A report that answered one axis and silently stayed blank on the other would be the same
shape of silent-wrong-answer this whole cluster exists to fix.

FilletResult gained an explicit public init: a let property with a default value is dropped
entirely from Swift's synthesized memberwise init (only a var with a default is exposed as an
overridable parameter -- confirmed by hand before relying on it, see the doc comment on the new
init), so the three existing WithReport call sites needed an explicit default to keep compiling
unchanged.

filletEvolving(_:) measures the identical last-wins mechanism (documented on
EvolvingFilletEdge's own doc comment, confirmed again by this PR's re-measurement), and is
deliberately left alone: extending filletEvolvingWithReport(_:) with the same field is a natural
follow-up, not a requirement of an issue scoped to blendedEdges(_:) by its own title.

Docs

• docs/SEMVER.md: recorded as additive, following The fillet family cannot tell a caller that OCCT declined some of the edges it named #639's own "not an exception" precedent --
  re-checked the counter arithmetic (three non-compiling + ten behavioural = thirteen sections) and
  confirmed this PR does not move it.
• docs/CHANGELOG.md, docs/API_REFERENCE.md (+2 to the derived operation count via
  Scripts/count-operations.py --fix: the new method, and the new explicit FilletResult.init the
  counter also picks up as a public entry point -- 4,304 to 4,306), README.md headline count.
• docs/reference/Shape-Features.md: Shape.FilletResult, blendedEdges(_:) and the new
  blendedEdgesWithReport(_:) entries.
• Scripts/repro/cluster-b-fillet-edge-contract/README.md and ClusterB.swift: the measured grid
  values did not move (still OVERWRITE/SKIP the same way), only the annotations on the now-fixed
  row, plus a paragraph correcting the "one contract, chosen and applied" framing to "report, not
  converge," matching what The fillet family cannot tell a caller that OCCT declined some of the edges it named #639 actually did for its own axis.

Test plan

• New test: Tests/OCCTModelingTests/Issue633BlendedEdgesDuplicateReportTests.swift, 7 cases,
  following okf/policies/prove-the-test-fails.md. Injection matrix (each restored and
  confirmed green afterward):

  Injection	Result
  The Swift-side helper drops the report (return [])	The 3 tests asserting a non-empty overwrittenDuplicateIndices fail; the 4 that expect it empty, or check declinedEdgeIndices only, correctly stay green
  The Swift-side helper deduplicates (Array(Set(overwritten))) instead of mirroring the request	Only the triple-duplicate test fails ([0, 0] collapses to [0]); the single-duplicate test is insensitive to this distinction and stays green
  OCCTShapeBlendEdges stops forwarding its two new out-parameters (nullptr, nullptr always)	The 2 tests asserting a non-empty declinedEdgeIndices fail; both duplicate-only tests, unaffected by this axis, correctly stay green

• Clean swift build, 0 errors, no new warnings, no OCCTSWIFT_LOCAL, no local Libraries/
  (pins v2.0.0-kernel.1).

• Full swift test, no env overrides: 5,363 tests passing (baseline 5,356 + 7 new), 2
  consecutive clean runs, no crash flake.

• swift run Censuses cluster-a 45 rows, cluster-b 16 rows -- unchanged.

• Four gate scripts (check-bridge-index.py, check-null-handle-guards.py,
  check-docs-defaults.py, count-operations.py) and the three --self-tests, from the repo
  root: all pass.

• Zero em-dashes (code, docs, this PR body).

Notes for the reviewer

• The fillet/chamfer first-wins/last-wins asymmetry itself is unchanged and not addressed here --
  see "The decision" above. Converging the two families is a real question with a real cost (a
  behaviour change on every existing caller of at least one family), and is deliberately left open
  rather than folded into an observability fix.
• overwrittenDuplicateIndices is computed entirely Swift-side; the only bridge change is
  threading The fillet family cannot tell a caller that OCCT declined some of the edges it named #639's existing declined-edge out-parameters through OCCTShapeBlendEdges, following
  the exact pattern already established for its three siblings.
• The FilletResult.init needing to be explicit (rather than relying on the synthesized
  memberwise init) surprised me -- verified with a standalone Swift snippet before writing the fix,
  since a let property's default silently not becoming an overridable init parameter is easy to
  assume works like a var's does.

[gsdali]

Independent verification

It took #709's recommendation rather than inventing a second reporting shape, which was the main risk here. FilletResult gains overwrittenDuplicateIndices alongside declinedEdgeIndices, so the family has one result type rather than two, and #490's lesson about parallel encodings of one idea holds.

Measured the behaviour myself:

one duplicate  -> overwritten = [0]        one loser
triple         -> overwritten = [0, 0]     two losers, matching the doc
no duplicate   -> overwritten = []
plain sibling  -> unchanged, still non-nil

The triple case is the one worth having: reporting per losing entry rather than per distinct edge is what makes the count mean "how many radii were discarded", and it is the semantics that a deduplicating implementation would silently get wrong.

My own injection, replacing the computed list with [], fails two tests with the exact expectation:

✘ a duplicated edge index reports one overwritten entry
     report.overwrittenDuplicateIndices → []  ==  [0]
✘ an edge named three times reports two overwritten entries, not one

Verified: full suite 5,363 tests, 0 failures, four gate scripts and their three --self-tests green, zero em-dashes.

One process note, not about this PR's content

The agent reported back twice without a PR URL, both times just saying it was waiting on CI. The work was complete and pushed both times. Worth knowing when reading the task log rather than the branch.

On SEMVER

I asked for a recorded-exception entry and a counter re-check when dispatching this. That instruction was wrong, and I have stopped giving it. The recorded-exception mechanism is for breaks shipping within a major line, which SEMVER.md says plainly; everything here ships in v2.0.0, a major, where breaking changes are simply permitted. What these entries are actually worth is the migration note, not the exception accounting. Nothing to change in this PR, but the accumulated Unreleased entries across the cluster work are really the v2.0.0 migration guide wearing the wrong label, and that is worth restructuring once the in-flight PRs land rather than while six agents are editing the file.

[secondmouseAU-bot]

Review

Scope reviewed: the full 11-file diff (bridge header/impl, Swift API, new test file, docs). I fetched the surrounding context this diff builds on — occtShapeFilletEdgeList/occtFilletWriteDeclined in OCCTBridge_Internal.h, the #639 sibling OCCTShapeFilletEdges, and filletedWithReport(edges:radius:) — to check this PR's changes against the established pattern rather than taking "follows the existing pattern" on faith. I also compiled a standalone Swift snippet to verify one specific language-semantics claim in the doc comments.

Overview

Adds blendedEdgesWithReport(_:), a WithReport sibling to blendedEdges(_:) that reports two things: which requested edges OCCT declined (adopting #639's existing mechanism for this one remaining entry point) and, new for this issue, which duplicate edgeRadii entries had their radius silently overwritten by a later entry naming the same edge index. Shape.FilletResult gains a second field (overwrittenDuplicateIndices) and an explicit public init rather than a new parallel result type. No existing method's behavior changes — blendedEdges(_:) is byte-for-byte unchanged.

Verified correctness

• Bridge-side change matches the established pattern exactly. I compared the modified OCCTShapeBlendEdges against OCCTShapeFilletEdges (the The fillet family cannot tell a caller that OCCT declined some of the edges it named #639 sibling) side by side: same guard/zero of outDeclinedCount, same delegation to occtShapeFilletEdgeList with the two out-params threaded straight through as trailing args. occtShapeFilletEdgeList's new params default to nullptr, so the pattern is additive at the C++ layer too.
• Buffer-safety contract checked, not assumed. occtFilletWriteDeclined's doc comment states the caller must size declinedEdgeIndices to at least edgeCount — there's no capacity parameter passed in to bound-check against. blendedEdgesWithReport(_:) allocates declined sized to exactly edgeRadii.count (== the count passed to the bridge call), matching the contract precisely; no overflow risk, and it's the identical sizing idiom filletedWithReport(edges:radius:) already uses.
• The duplicate-detection algorithm is correct. I hand-traced overwrittenDuplicateIndices(in:) against the triple-duplicate case from the PR's own test ([(0,2.0),(0,3.0),(0,5.0)]): lastPosition[0] ends at index 2 (dictionary overwrite), so positions 0 and 1 both fail the != position check and are reported, position 2 (the winner) isn't — yielding [0, 0], matching the test's expectation.
• The let-with-default memberwise-init claim is real — verified with the compiler, not just read. I compiled:

  struct Foo { let a: Int; let b: [Int] = [] }
  let f1 = Foo(a: 1)             // compiles — b silently uses its default
  let f2 = Foo(a: 1, b: [9, 9])  // error: extra argument 'b' in call

  Confirmed: a let with a default is dropped from the synthesized memberwise init's parameter list entirely — you can't pass it at all, not even to keep the default. So blendedEdgesWithReport(_:)'s call, which must override overwrittenDuplicateIndices away from [], could not compile without the explicit init. The explicit init is genuinely required.

One finding: the new init's doc comment misattributes why it's needed

The doc comment reads: "...so the three existing WithReport call sites... need overwrittenDuplicateIndices to keep defaulting to empty without themselves changing." That's backwards from what I verified above — those three existing call sites (filletedWithReport(edges:radius:), filletedWithReport(edges:startRadius:endRadius:), filletEvolvingWithReport(_:)) would have compiled identically fine even relying on the synthesized init alone, since a dropped let-with-default parameter just silently uses [] regardless of whether an explicit init exists — my f1 case above. The actual reason the explicit init is required is the new caller: blendedEdgesWithReport(_:) needs to pass a non-default value, and a synthesized init structurally cannot accept an override for a defaulted let property at all (my f2 case). The code is correct either way — the explicit init is the right fix — but the comment's stated justification points at the callers that were never at risk instead of the one that actually forced the change. Worth a one-line correction.

Minor / disclosed-scope notes (not defects)

• filletEvolving(_:) shares the identical last-wins duplicate-overwrite mechanism (per EvolvingFilletEdge's own doc comment) but filletEvolvingWithReport(_:) isn't extended here — explicitly and reasonably scoped out as a follow-up rather than silently left inconsistent. Flagging only for visibility since the same silent-discard shape this issue exists to fix still lives, unaddressed, in a sibling method.
• Fillet/chamfer first-wins vs. last-wins asymmetry across the wider family remains deliberately out of scope, consistent with The fillet family cannot tell a caller that OCCT declined some of the edges it named #639's own precedent of "report, don't converge" — already well disclosed in the PR body and docs, no action needed.

Code quality / conventions / performance / security / test coverage

• Extending FilletResult rather than introducing a parallel result type is the right call and matches the project's own Consolidate the bridge's ~8 duplicate int→GeomAbs_Shape continuity mappers — divergent numbering already shipped one bug (#433) and still causes bsplineRestriction vs bsplineRestrictionAdvanced to silently disagree #490 precedent, cited correctly.
• Buffer handling on the Swift side (withUnsafeMutableBufferPointer nesting, prefix(Int(count)).map(Int.init) truncation) is byte-for-byte the same idiom as the pre-existing filletedWithReport, so no new unsafe-pointer risk profile introduced.
• Security: no buffer overflow risk (verified against the bridge helper's sizing contract above); no other user-facing attack surface.
• Performance: the new Swift-side dedup pass is O(n) over the request list (bounded by edge count); the bridge-side addition only activates when the two new out-params are non-null, so blendedEdges(_:) pays nothing extra.
• Test coverage: 7 new tests with an explicit injection matrix (drop the report / dedupe instead of mirror / stop forwarding declined out-params), each independently caught by a distinct subset of tests — good discipline. I verified the pure-Swift dedup logic by hand-trace rather than by running the suite (no OCCT kernel available in this environment), so I can't independently confirm the OCCT-side (declinedEdgeIndices) assertions beyond checking the code path is structurally identical to the already-shipped The fillet family cannot tell a caller that OCCT declined some of the edges it named #639 mechanism.

Bottom line

Solid, low-risk change: it reuses an established mechanism correctly, the one non-obvious Swift-semantics claim it makes checks out under an actual compiler, and the buffer-safety contract holds. The only thing worth fixing before/after merge is the inverted reasoning in the new init's doc comment — cosmetic, not a functional issue.

---

🤖 Posted by Claude Code review

[gsdali]

Fixed. The comment pointed at the three existing WithReport call sites as the reason for the explicit init; they would have compiled unchanged against the synthesized one, silently taking the [] default. The real forcing case is blendedEdgesWithReport, the first caller needing a non-default value, which a synthesized init structurally cannot express. Correct fix, wrong stated cause.

Thanks for compiling the f1/f2 pair rather than reading the language rule off documentation. That is the check that distinguishes "dropped from the parameter list" from "present with a default", and it is exactly what made the misattribution visible.

Not acted on, and I agree with your framing: filletEvolvingWithReport shares the identical last-wins mechanism and is not extended here. It is disclosed rather than silently inconsistent, and it wants the same treatment in a follow-up.

7 tests pass, four gate scripts green, zero em-dashes.