feat(#772): opt-in self-intersection check for analyze(tolerance:) - #773
Conversation
Measured Shape.analyze(tolerance:) against isSelfIntersecting(timeout:) across a box, a moderately complex fused/filleted solid, a real 662-face mesh-sewn import, and the #319 pathological artifact. Overhead is 1x-3x on every ordinary shape but ~1800x-4100x on the pathological one (a few ms vs 30+s), so the check is opt-in (checkSelfIntersection: false by default), not unconditional. analyze(tolerance:checkSelfIntersection:hardTimeout:) adds the opt-in path; ShapeAnalysisResult.hasSelfIntersection is Bool? (not Int?, since the underlying check never answers with a count), non-nil exactly when requested and resolved, proven reachable-and-populated by two new tests per #771's warning against an always-closed gate. See Scripts/repro/772-analyze-self-intersection/ for the full measurement.
|
The measurement is convincing and the decision follows from it. One thing sent back, in The decision is right and the numbers carry it
Negligible on everything ordinary, catastrophic on one real artifact. That is exactly the shape that
#771's constraint is met properly. What needs fixingThe new
A manifest path into a per-cluster directory couples The PR's own comment justifies the excludes as working "the same way Sent back to place the source where no manifest change is needed, either as another Worth notingThis is the second time today a change has been justified against a real precedent that was not the |
Review found that the AnalyzeSelfIntersectionTiming executableTarget added in the previous commit reintroduced both halves of the pattern #694 deliberately removed: a manifest path into a per-issue repro directory (coupling swift build to a directory name that does get renamed) and a second exclude: list to maintain. Moved the Swift source into a new shared Harnesses target (Scripts/repro/harnesses/), structured the same way Censuses is: HarnessRunner.swift dispatches by name (swift run Harnesses 772-self-intersection), and the harness logic itself is an AnalyzeSelfIntersectionTiming.swift with an enum { static func run() } entry point and fileprivate helpers, matching ClusterA/B/D's shape. Scripts/repro/772-analyze-self-intersection/ now holds only its README and captured output, no exclude: needed. Updated both files' run instructions to match. Verified: swift build --target Harnesses has zero warnings (no invalid exclude entries), swift run Harnesses 772-self-intersection reproduces the same measurements, all 5 gate scripts + self-tests clean, full swift test (5487 tests, 1429 suites) passes.
secondmouseAU-bot
left a comment
secondmouseAU-bot
left a comment
There was a problem hiding this comment.
Automated review (medium effort): 5 findings — see inline comments. The two most significant (line 233, 270) are about the shipped checkSelfIntersection path calling isSelfIntersecting(hardTimeout:), a materially heavier mechanism (deepCopy + background dispatch + semaphore, confirmed against Shape.swift) than the one actually benchmarked on ordinary shapes (isSelfIntersecting(timeout:)) — the measurement backing "cheap enough to opt into freely" doesn't cover the code path that ships. The other three are a silent-inert-parameter footgun (hardTimeout without checkSelfIntersection: true), a missing blocking-thread warning on the new analyze() overload, and a duplicated-not-shared harness runner. Design/decision is sound overall — this is about the doc/measurement precision and a couple of footguns around it, not the opt-in-vs-default-on call itself.
| /// - tolerance: Size threshold for detecting small features. | ||
| /// - checkSelfIntersection: If `true`, also runs ``isSelfIntersecting(hardTimeout:)`` and | ||
| /// populates ``ShapeAnalysisResult/hasSelfIntersection``. Default `false`, because that | ||
| /// check is orders of magnitude more expensive than the rest of this scan and, on |
There was a problem hiding this comment.
The 1x-3x / 1.2x-3.2x overhead figures behind "cheap enough to opt into whenever a caller actually wants the answer" were measured against isSelfIntersecting(timeout:) (AnalyzeSelfIntersectionTiming.swift's report(), used for rows 1-3), but this parameter actually wires into isSelfIntersecting(hardTimeout:) (line 270) — confirmed a materially different, more expensive mechanism than timeout:: timeout: is a direct synchronous OCCTShapeSelfIntersectsBounded(handle, timeout) call, while hardTimeout: does deepCopy() + DispatchQueue.global(qos: .userInitiated).async + DispatchSemaphore.wait (Shape.swift:2150-2169).
The only place hardTimeout: is actually measured in this PR's own harness is row "4b", and only on the #319 pathological artifact — never on an ordinary shape. For a caller trusting this doc's cost claim on a large-but-ordinary import (e.g. the 662-face mesh-sewn fixture), the real cost — including the deep copy of the whole shape's geometry — was never benchmarked.
| public func analyze(tolerance: Double = 1e-6, checkSelfIntersection: Bool = false, | ||
| hardTimeout: Double = 30) -> ShapeAnalysisResult? { | ||
| let result = OCCTShapeAnalyze(handle, tolerance) | ||
| guard result.isValid else { return nil } |
There was a problem hiding this comment.
analyze(checkSelfIntersection: true) synchronously blocks the calling thread for up to hardTimeout (default 30s) via the DispatchSemaphore.wait inside isSelfIntersecting(hardTimeout:) (Shape.swift:2161) — but analyze()'s own doc comment (lines 231-239 above) never says "blocks the calling thread" or warns against calling it from the main/UI thread, only that the check is "orders of magnitude more expensive."
isSelfIntersecting(hardTimeout:) itself does carry that warning implicitly via its "detached background thread... waits on the calling thread with a real deadline" doc, but analyze() reads as the routine validation entry point — a more likely main-thread call site than the more cautiously-scoped isSelfIntersecting escape hatch — and doesn't inherit or repeat that warning. On a moderately pathological shape this can freeze a UI thread for up to 30s with no warning at the call site actually used.
| let result = OCCTShapeAnalyze(handle, tolerance) | ||
| guard result.isValid else { return nil } | ||
|
|
||
| let hasSelfIntersection: Bool? = checkSelfIntersection |
There was a problem hiding this comment.
hardTimeout is completely inert unless checkSelfIntersection is also true — confirmed, it's referenced nowhere else in the function body. A caller who calls analyze(hardTimeout: 5) by label, forgetting checkSelfIntersection: true (which defaults false and sits between tolerance and hardTimeout in the signature), compiles and runs fine — hasSelfIntersection is simply nil, indistinguishable from the normal default-off case, and the 5s deadline is silently discarded with no compiler or runtime signal.
| guard result.isValid else { return nil } | ||
|
|
||
| let hasSelfIntersection: Bool? = checkSelfIntersection | ||
| ? isSelfIntersecting(hardTimeout: hardTimeout) |
There was a problem hiding this comment.
isSelfIntersecting(hardTimeout:) abandons (does not cancel) its background computation when hardTimeout elapses — confirmed in its own doc comment: "the background computation is abandoned, not cancelled — it keeps running orphaned on its own thread until it eventually completes" (Shape.swift:2122-2125). That behavior isn't new, but this PR wires it into a much more commonly-called entry point than the narrower isSelfIntersecting escape hatch.
Code that loops shape.analyze(checkSelfIntersection: true, hardTimeout: <small>) over many imported files — a natural usage pattern for a general analysis API — accumulates abandoned background computations, each burning CPU and holding a full deepCopy()'d probe shape alive, with no cancellation and no bound on how many can pile up concurrently.
| ] | ||
|
|
||
| static func main() { | ||
| let arguments = Array(CommandLine.arguments.dropFirst()) |
There was a problem hiding this comment.
HarnessRunner.main()/printUsage() duplicates Scripts/repro/censuses/CensusRunner.swift's dispatch logic almost line for line (same nil/"list"/"all"/named-lookup switch, same stderr-write-then-printUsage-then-exit(1) error path, same column-padding usage formatter, differing only in the padding width and the struct/registry names) instead of factoring both into one generic runner. The file's own header comment even names the parallel ("same shared-target shape as the Censuses target's own CensusRunner.swift") without extracting it.
A future fix to the dispatch/list/error-handling logic (a new flag, a wording change) has to be manually ported to both CensusRunner.swift and HarnessRunner.swift or the two runners silently drift apart — the same per-target duplication #694 was originally meant to eliminate by consolidating into shared executable targets in the first place.
…rdTimeout: Review found the timing harness measured isSelfIntersecting(timeout:) on ordinary shapes while analyze(tolerance:checkSelfIntersection:hardTimeout:) actually wired into isSelfIntersecting(hardTimeout:), a different and more expensive mechanism (deepCopy + DispatchQueue.global + DispatchSemaphore, with an internally UNBOUNDED OCCTShapeSelfIntersectsBounded call, timeout 0) never measured on anything but the pathological artifact. Re-measured deepCopy() and both self-intersection entry points, at the same deadline, on every row (Scripts/repro/harnesses/AnalyzeSelfIntersectionTiming.swift). deepCopy() is cheap everywhere (under 1ms, including the 662-face import). But on the #319 pathological artifact, hardTimeout: reliably returned nil (indeterminate) at the deadline while timeout: reliably returned a conclusive true at the same wall-clock cost: the same budget for a worse answer, because hardTimeout:'s internal call has no cooperative deadline of its own. analyze() is already fully synchronous, so hardTimeout:'s wall-clock guarantee buys nothing over timeout: in that context. analyze(tolerance:checkSelfIntersection:hardTimeout:) is now analyze(tolerance:selfIntersectionTimeout:), forwarding to isSelfIntersecting(timeout:). Collapsing the two old parameters into one optional also fixes review's finding 3: a caller could previously supply hardTimeout without checkSelfIntersection: true and have it silently discarded; supplying a value now IS opting in, so that mistake no longer compiles into anything but the intended behavior. Doc comment now states explicitly that this blocks the calling thread (finding 2). Finding 4 (abandoned background computations piling up) is resolved by construction, not documentation, since analyze() no longer uses the hardTimeout: mechanism at all. Also factors HarnessRunner's dispatch logic (list/all/run-by-name, error handling, usage printing) into a new shared RunnerCore library target, consumed by both Harnesses and Censuses, since HarnessRunner had reproduced CensusRunner almost line for line instead of sharing it (finding 5). New test: timeoutAloneCannotBeSuppliedWithoutOptingIn, the regression test for the collapsed-parameter fix. Existing tests renamed to match the new parameter name.
secondmouseAU-bot
left a comment
secondmouseAU-bot
left a comment
There was a problem hiding this comment.
Automated second-pass review (medium effort): 5 findings — see inline comments. Round 2's rewrite genuinely fixed everything round 1 flagged (verified: analyze() now calls isSelfIntersecting(timeout:) not hardTimeout:, the checkSelfIntersection/hardTimeout footgun is collapsed into one selfIntersectionTimeout: Double?, and the HarnessRunner/CensusRunner duplication is gone via a real shared RunnerCore/GenericRunner target). No correctness bugs found this pass. The two most worth attention: an efficiency short-circuit opportunity (self-intersection check runs even when hasInvalidTopology is already true) and a rigor gap (the prove-the-test-fails removal matrix wasn't actually re-run against round 2's code for the "always-check" defect class, only round 1's). The other three are minor cleanup (a vestigial comparison, a 4th duplicated test fixture, a duplicated ratio calculation).
| let result = OCCTShapeAnalyze(handle, tolerance) | ||
| guard result.isValid else { return nil } | ||
|
|
||
| let hasSelfIntersection: Bool? = selfIntersectionTimeout.flatMap { isSelfIntersecting(timeout: $0) } |
There was a problem hiding this comment.
The self-intersection check runs unconditionally whenever selfIntersectionTimeout is non-nil, even when result.hasInvalidTopology (checked two lines later) is already true.
Failure scenario: batch-analyzing a folder of imports with selfIntersectionTimeout: 30, several files with grossly broken topology still pay the full BOPAlgo_ArgumentAnalyzer pass (up to 30s each) even though hasInvalidTopology already answers the health question near-instantly. Worth considering short-circuiting hasSelfIntersection to nil when topology is already known invalid.
| func overlappingCompound() -> Shape { | ||
| let a = Shape.box(origin: SIMD3(0, 0, 0), width: 10, height: 10, depth: 10)! | ||
| let b = Shape.box(origin: SIMD3(5, 0, 0), width: 10, height: 10, depth: 10)! | ||
| return Shape.compound([a, b])! |
There was a problem hiding this comment.
The PR's prove-the-test-fails removal matrix only re-verified the always-nil-gate injection against round 2's code; the complementary "unconditionally computes regardless of nil timeout" defect class was only run against round 1's now-replaced API, per the PR body's own text ("Round 1's always-check injection was re-verified against round 1's implementation... see the prior review thread").
The checklist claims "every new test was run once with its subject broken" as proof — that specific proof for the always-check defect was never actually re-run on the code that ships in round 2, even though defaultDoesNotCheck would likely still catch it in practice. Worth a quick re-run to actually back the claim.
| print("Available:") | ||
| let width = entries.map(\.name.count).max() ?? 0 | ||
| for entry in entries { | ||
| let name = entry.name.count >= width ? entry.name : entry.name + String(repeating: " ", count: width - entry.name.count) |
There was a problem hiding this comment.
entry.name.count >= width is a vestigial comparison carried over from the old hardcoded-12 CensusRunner code; width is now entries.map(\.name.count).max() ?? 0, so count can never exceed it — the > half of >= is unreachable.
Not a bug today, but it implies a case that can't occur post-refactor, which can mislead a future reader. Suggest simplifying to entry.name.count == width.
| /// #319 pathological artifact, at the same wall-clock cost; `analyze()` now forwards to | ||
| /// `timeout:` instead. (2) the two parameters that gated the check | ||
| /// (`checkSelfIntersection: Bool`, `hardTimeout: Double`) let a caller supply a timeout while | ||
| /// forgetting the boolean, compiling and silently discarding it; they are now one |
There was a problem hiding this comment.
overlappingCompound() hand-builds a 4th independent copy of the "two boxes offset by 5, wrapped in a compound" self-intersecting fixture already duplicated twice in Issue208SelfIntersectionTests.swift, despite this file's own comment claiming reuse of that construction.
Failure scenario: if the shared self-intersecting fixture ever needs to change (tolerance, box size), a maintainer has to find and update 4 independent copies instead of one shared helper. Consider extracting a shared fixture helper (the repo already has precedent for this, e.g. SharedFixture.swift).
| analyzeSeconds: analyzeSeconds, deepCopySeconds: deepCopySeconds, | ||
| timeoutSeconds: timeoutSeconds, timeoutOutcome: describe(timeoutOutcome), | ||
| hardTimeoutSeconds: hardTimeoutSeconds, hardTimeoutOutcome: describe(hardTimeoutOutcome))) | ||
|
|
There was a problem hiding this comment.
The timeout/analyze overhead ratio is computed twice from the same stored row fields — once in report() for the per-row print, again in run()'s summary-table loop — instead of computed once and stored.
Failure scenario: a future change to the overhead formula (rounding, a different guard) has two call sites to update in lockstep, the exact duplication risk the RunnerCore extraction elsewhere in this same PR was written to eliminate. Suggest adding an overheadTimeout: Double field to SelfIntersectionTimingRow, computed once.
Three of the second-pass review's five findings taken. **The rigor gap is the real one.** The prove-the-test-fails matrix had re-run only the always-nil injection against round 2's code; the complementary always-check injection was verified against round 1's now-replaced API, and the PR body said so while the checklist still claimed every test had been run with its subject broken. Re-run against the shipped code: injecting `isSelfIntersecting(timeout: selfIntersectionTimeout ?? 30)` in place of the `flatMap` fails 2 tests with 3 issues, restored byte-identical, all pass. The claim is now backed by the code that ships. `GenericRunner`'s `entry.name.count >= width` is vestigial: `width` is the max of those counts, so the `>` half cannot occur. Narrowed to `==`. The timeout overhead ratio was derived in two places from the same fields. Now one computed property on `SelfIntersectionTimingRow`, which is the duplication the `RunnerCore` extraction in this same PR exists to prevent. **Two findings rejected, with reasons rather than silence.** Short-circuiting the self-intersection check when `hasInvalidTopology` is already true would make `hasSelfIntersection == nil` mean two things: not requested, and requested but skipped. That is exactly the collision issue #726 exists to remove and that `hasExtent` (#771) turned out to be. A caller who passes a timeout asked for the answer, and invalid topology does not make the answer uninteresting. Extracting a shared self-intersecting fixture is not the convention here. CLAUDE.md's Test Layout says each target is a separate module and helpers are redefined per target; `Issue208SelfIntersection` is in `OCCTModelingTests` and this suite is in `OCCTShapeHealingTests`. The review also cited `SharedFixture.swift` as precedent, and no such file exists anywhere in the repo. Within this file the fixture is already one helper used three times.
|
Three taken, two rejected with reasons. TakenThe rigor gap was the real finding. The matrix had re-run only the always-nil injection against
The overhead ratio is now one computed property on RejectedShort-circuiting when Extracting a shared self-intersecting fixture. Two problems. CLAUDE.md's Test Layout says each That second one is worth flagging as a review-quality note rather than a complaint: a finding whose Full suite 5488 tests, gates clean. Merging. |
PR #773's `RunnerCore` extraction renamed `CensusEntry` to `RunnableEntry`, and this branch had added an `issue-761` row using the old name. One conflict, one hunk: took the base's type and re-added the row on it. Verified rather than assumed, since a type rename that compiles can still have dropped a row: `swift run Censuses issue-761` still dispatches and reports the same 0.4 / 2.6 ms it did before the merge, and the full suite is 5495 tests green.
2 participants
What & why
#772 asks whether
Shape.analyze(tolerance:)should measure self-intersection now thatShapeAnalysisResult.selfIntersectionCount(always 0, never computed) is gone (#763/PR#770). Theissue is explicit that this is a measure-first decision, not a pick-the-option-that-sounds-right
one (
okf/policies/measure-dont-assume.md).This PR went through two measurement rounds; the second one changed the design. Round 1
measured
isSelfIntersecting(timeout:)on ordinary shapes and shipped a parameter that actuallycalled
isSelfIntersecting(hardTimeout:), a different mechanism never measured except on thepathological artifact. Review caught this. Round 2 measures both entry points, plus
deepCopy()alone, at the same deadline, on every row, and the numbers changed which mechanism
analyze()uses. Full harness and captured output at
Scripts/repro/772-analyze-self-intersection/(
README.md,measured-output.txt); the harness itself isScripts/repro/harnesses/AnalyzeSelfIntersectionTiming.swift, one entry in the sharedHarnessesexecutable target (Package.swift,swift run Harnesses 772-self-intersection).Round 2 measurement (current, correct)
analyze(tolerance:)deepCopy()timeout: 30(shipped)hardTimeout: 30(rejected)Two findings decide the design:
deepCopy()is cheap everywhere (under 1ms, including the 662-face import), so it wasnever really the cost concern review named. Both mechanisms cost about the same on ordinary
shapes (0.8x-2.0x vs
analyze()'s own baseline scan).hardTimeout:gives a worse answer thantimeout:for thesame wall-clock cost.
timeout:reliably returns a conclusiveself-intersectsaround30.05-30.15s;
hardTimeout:reliably returnsnil(indeterminate) at almost exactly 30.0s,every run. Structural, not incidental:
hardTimeout:'s internalOCCTShapeSelfIntersectsBoundedcall passes
0(unbounded), so the background computation has no cooperative deadline of itsown to find the fault before the caller's semaphore gives up;
timeout:'s own internalcheckpoint-based breaker does.
hardTimeout:also leaves that computation running, abandonedand still unbounded, after returning
nil.Decision:
analyze(tolerance:selfIntersectionTimeout:)forwards toisSelfIntersecting(timeout:), notisSelfIntersecting(hardTimeout:).analyze()is already afully synchronous call with no async variant, so a caller has already committed to blocking;
hardTimeout:'s wall-clock guarantee buys nothing overtimeout:in that context, and on the oneartifact where it mattered it cost a worse answer at the same price, plus an abandoned background
computation. A caller who genuinely needs the hard guarantee should call
isSelfIntersecting(hardTimeout:)directly and accept its documented trade-offs.The opt-in-vs-default-on question from round 1 is unchanged by round 2's correction: cheap
(1x-2x) on every ordinary shape but ~3500x-4200x on the pathological one (a real reconstruction
pipeline artifact, not a contrived case) still means an unconditional or default-on check would
occasionally turn a cheap call into a many-second one, silently.
ShapeAnalysisResult.hasSelfIntersectionis
Bool?(not theInt?the issue's option-3 sketch suggested, sinceisSelfIntersectingneveranswers with a count), non-nil exactly when
selfIntersectionTimeoutwas non-niland the checkresolved, satisfying #771's bar against an always-closed gate.
Full reasoning and both rounds' numbers:
Scripts/repro/772-analyze-self-intersection/README.md.Baseline note: branched from
origin/refactor/381-pass1bbefore PR #770 merged (baseline was809f148,selfIntersectionCountstill present); rebased ontoad1078e(post-#770 merge) once#770 landed mid-task, dropping every reference to the now-removed field so this PR does not
reintroduce it.
Closes #772
CHANGELOG entry
Shape.analyze(tolerance:)can now check self-intersection (opt-in) (#772)analyze(tolerance:selfIntersectionTimeout:)gains one new parameter,selfIntersectionTimeout: Double? = nil.nil(the default) skips the self-intersection check entirely; a non-nilvalueopts in, forwarded as the
timeout:toisSelfIntersecting(timeout:)(the sameBOPAlgo_ArgumentAnalyzercheckisSelfIntersectinguses), and populates the newShapeAnalysisResult.hasSelfIntersection: Bool?field:nilwhen not requested, or requested butindeterminate;
true/falsewhen the check resolved.totalProblemsadds a flat +1 whenhasSelfIntersection == true, matching howhasInvalidTopologyis counted.Passing a non-
nilselfIntersectionTimeoutmakes this call block the calling thread for upto that many seconds (more, if OCCT never reaches a checkpoint to poll); do not pass it from a
UI/main thread without accepting that stall.
Measured before deciding (
Scripts/repro/772-analyze-self-intersection/): the check costs 1x-2xthe rest of
analyze()'s scan on ordinary shapes but ~3500x-4200x on a known pathologicalartifact (a few ms vs 30+ seconds), so it defaults off rather than running unconditionally.
SemVer impact
MINOR. Purely additive:
analyze(tolerance:selfIntersectionTimeout:)adds one new parameter witha default, so every existing call site keeps compiling and keeps its current behavior unchanged.
ShapeAnalysisResultgains a new field (hasSelfIntersection: Bool?); no existing field changedtype or was removed, and the struct's memberwise initializer is not public, so no external caller
constructs one directly. No migration needed.
Checklist
Tests/OCCTShapeHealingTests/Issue772SelfIntersectionAnalysisTests.swift(5 tests, oneadded in round 2 specifically for the collapsed-parameter fix), plus one existing test
(
analysisResultProperties) updated to include the new field in its ownindependently-recomputed
totalProblemsmirror.docs/CHANGELOG.mdis not in this diff.docs/SEMVER.mdis not in this diff.Notes for the reviewer
Round 1 packaging fix: the first pushed commit added the harness as its own
AnalyzeSelfIntersectionTimingexecutable target, path pointing directly intoScripts/repro/772-analyze-self-intersection/, withexclude: ["README.md", "measured-output.txt"], reintroducing what #694 removed one-target-per-cluster for. Moved theSwift source into a new shared
Harnessestarget (Scripts/repro/harnesses/).Round 2, five findings, all fixed:
isSelfIntersecting(timeout:)forordinary shapes while
analyze()actually calledisSelfIntersecting(hardTimeout:).Re-measured both mechanisms, plus
deepCopy()alone, on every row, at the same deadline (seetable above). Result: switched
analyze()to forward totimeout:instead, sincehardTimeout:is not a strict improvement (same cost on ordinary shapes, a worse answer onthe pathological one) and buys nothing for an already-synchronous caller.
analyze()'s doc now has an explicit- Importantnote: passingselfIntersectionTimeoutblocks the calling thread for up to that many seconds, matchingisSelfIntersecting(timeout:)'s own warning.checkSelfIntersection: Bool+hardTimeout: Doublelet a caller supply a timeout while forgetting the boolean, compiling and discarding it
silently. Collapsed into one
selfIntersectionTimeout: Double?: supplying a value now ISopting in, making that mistake unrepresentable. New test:
timeoutAloneCannotBeSuppliedWithoutOptingIn.analyze()no longer usesisSelfIntersecting(hardTimeout:)at all (per finding 1's fix), thespecific risk of piling up abandoned, deep-copied probe shapes across a loop of
analyze()calls does not reach
analyze().isSelfIntersecting(hardTimeout:)itself is unchanged andstill carries that documented trade-off for any caller who reaches it directly (out of scope
for analyze() no longer reports self-intersection at all: decide whether it should measure it #772, per the issue's own "Not in scope" section).
HarnessRunnerduplicatedCensusRunner. Factored the shared dispatch logic(list/all/run-by-name, error handling, usage printing) into a new
RunnerCorelibrary target(
Scripts/repro/runner-core/GenericRunner.swift), which bothHarnessesandCensusesnowdepend on. One manifest edit, no per-target duplication left.
Removal matrix (prove-the-test-fails,
okf/policies/prove-the-test-fails.md), re-run againstthe final round-2 implementation:
hasSelfIntersectionforced tonilunconditionally): 4 of 5 tests FAILED(
nonNilTimeoutOnCleanShapePopulatesFalse,nonNilTimeoutOnSelfIntersectingShapePopulatesTrue,totalProblemsReflectsOnlyWhatWasChecked,timeoutAloneCannotBeSuppliedWithoutOptingIn); thedefault-stays-nil test still passed (orthogonal to this defect). Restored (
diffconfirmedbyte-identical to pre-injection), re-ran: all 5 pass.
round-2 rewrite; see the prior review thread for that run's output.)
Gates: all 5 static gates + their
--self-tests, plus the census and changelog-transcriptionself-tests, all clean after both rounds.
count-operations.py: 4306 == 4306 (unaffected; asignature/parameter change, not a new entry point).
Full
swift test: 5488 tests, 1429 suites, 0 failures, ~113s wall clock, after round 2.Docs:
docs/reference/Shape-Features.md'sShapeAnalysisResult/analyze(...)sectionsupdated with the new field/parameter, a fenced
swiftsnippet, the measured overhead figures, andthe
timeout:vshardTimeout:reasoning (context7 harvests the snippets).