From 5684b91da21a721e51c74ab3b5919eca6e23c624 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Mon, 10 Aug 2026 16:40:48 -0400 Subject: [PATCH] =?UTF-8?q?VV:=20Group=20MicroTexture=20Regions=20?= =?UTF-8?q?=E2=80=94=20READY=20FOR=20REVIEW?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the outstanding gate items left open by the initial V&V cycle (#1637) and promotes the report DRAFT -> READY FOR REVIEW. Summary: - Confirmed no new bugs (D1/D2 were found and fixed in the #1637 cycle; no SIMPLNX source change required by this pass); - documented 0 new deviations from DREAM3D 6.5.171, but corrected the existing 3 (D1 default-mode error, D2 randomization disabled, D3 legacy phase1 bug): the legacy comparison is reclassified "Not run" -> "N/A — no public legacy equivalent" because GroupMicroTextureRegions ships in 6.5.171 as a _PrivateFilter (ADD_SIMPL_FILTER(... FALSE ...)), compiled but not instantiable from a pipeline, so no A/B is possible; D2 and D3 "affected users" were overstated on that basis and are now accurate; - no tests retired (fresh gate-closing pass, not a re-verification); - augmented the existing 5 inlined *Class 1 (Analytical) + Class 4 (Invariant)* fixtures with no new cases; corrected a self-contradictory comment in the Tolerance Boundary fixture and documented that its expectation is seed-order independent; - added 3 V&V source-tree deliverables (report, deviations, provenance) — already present from #1637, updated in place by this pass; - fixed a coverage-gate violation: "9 of 9 paths exercised" contradicted two rows marked "Not directly tested"; corrected to 7 of 9 in both the dashboard row and the Code path coverage section. Notes for the reviewer: - The three deliverables disagreed on the UseRunningAverage=false comparison target. The provenance sidecar was correct; the report and the test comment were not. Verified against the source that determineGrouping() receives groupList[j] — the current BFS frontier feature, not the seed — so grouping is the transitive closure of the pairwise-tolerance relation along neighbor chains. Report and test comment now match the sidecar. - OOC verification is deliberately out of scope for this round: not required by the MTR SBIR deliverable and owned separately by another engineer. - Status stops at READY FOR REVIEW; per vv_policy.md the promotion to COMPLETE and the Sign-off field are filled at merge by the approving reviewer. Co-Authored-By: Claude Opus 5 --- .../test/GroupMicroTextureRegionsTest.cpp | 19 ++++++++++---- .../vv/GroupMicroTextureRegionsFilter.md | 26 ++++++++++++------- .../GroupMicroTextureRegionsFilter.md | 4 +-- .../GroupMicroTextureRegionsFilter.md | 16 +++++++++++- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/Plugins/OrientationAnalysis/test/GroupMicroTextureRegionsTest.cpp b/src/Plugins/OrientationAnalysis/test/GroupMicroTextureRegionsTest.cpp index ba564c2b94..8d74e7c1d5 100644 --- a/src/Plugins/OrientationAnalysis/test/GroupMicroTextureRegionsTest.cpp +++ b/src/Plugins/OrientationAnalysis/test/GroupMicroTextureRegionsTest.cpp @@ -393,12 +393,21 @@ TEST_CASE("OrientationAnalysis::GroupMicroTextureRegionsFilter: Class 1 Analytic using namespace AnalyticalFixtures; // 3 real features on a chain F1 -- F2 -- F3. F1 c-axis at Phi=0, F2 at Phi=8, F3 at Phi=20. - // - F1 -- F2 : 8 deg -- under 10 deg tolerance -> GROUP - // - F2 -- F3 : 12 deg -- over 10 deg tolerance using F2's c-axis (since UseRunningAverage=false, - // the algorithm compares each candidate to the BFS seed's c-axis, but - // inside the BFS walk over already-grouped features, comparison is from - // THAT feature's c-axis, not the original seed's) -> DO NOT BRIDGE + // + // With UseRunningAverage=false, determineGrouping() computes the reference c-axis from its + // referenceFeature argument, which execute() supplies as groupList[j] -- the current BFS frontier + // feature, NOT the seed and NOT a running average. (The local is named firstFeature, but groupList + // grows as neighbors are accepted, so it advances past the seed.) Grouping is therefore the + // transitive closure of the pairwise-tolerance relation along neighbor chains. + // + // - F1 -- F2 : 8 deg, compared from F1's c-axis -- under 10 deg tolerance -> GROUP + // - F2 -- F3 : 12 deg, compared from F2's c-axis -- over 10 deg tolerance -> DO NOT BRIDGE + // // Expected: 2 distinct groups -> {F1, F2}, {F3}. + // + // This expectation is independent of which feature getSeed() happens to pick first (it draws + // randomly among unparented features). Seeding at F1, F2, or F3 all produce the same partition, + // because the only bridging edge (F2--F3) exceeds tolerance when evaluated from either end. FixtureData td = CreateScaffold(/*numFeatures=*/4); SetAvgQuat(td, 1, QuatFromPhiDeg(0.0f)); diff --git a/src/Plugins/OrientationAnalysis/vv/GroupMicroTextureRegionsFilter.md b/src/Plugins/OrientationAnalysis/vv/GroupMicroTextureRegionsFilter.md index 06e72743b2..863d69bd4e 100644 --- a/src/Plugins/OrientationAnalysis/vv/GroupMicroTextureRegionsFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/GroupMicroTextureRegionsFilter.md @@ -8,8 +8,8 @@ | DREAM3D 6.5.171 equivalent | `GroupMicroTextureRegions` — `Source/Plugins/Reconstruction/ReconstructionFilters/GroupMicroTextureRegions.{h,cpp}` (inherits from `GroupFeatures` base class) | | Patched-legacy reference | 2025-10-23 surgical fix to a local build of the legacy source (`BUG: GroupMicrotextureRegions bug fixes, expose as usable filter`) — used here as a corroborating reference for port-time fixes | | Verified commit | ** | -| Status | DRAFT | -| Sign-off | *Michael Jackson — V&V pending review* | +| Status | READY FOR REVIEW | +| Sign-off | *pending second-engineer review* (V&V authored by Michael A. Jackson ) | ## At a glance @@ -17,12 +17,12 @@ |------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | Algorithm Relationship | **Port** (with deliberate inheritance flattening and 2 port-time **regressions fixed during this V&V cycle**, plus 1 latent legacy bug already corrected in the original 2024-01-08 port). Legacy is a `GroupFeatures` subclass; SIMPLNX inlines the `GroupFeatures::execute` BFS loop into a single algorithm class. UUID retained: `5e18a9e2-…` → `3f695987-…`. | | Oracle (confirmed) | **Class 1 (Analytical) primary + Class 4 (Invariant) companion** — pure Bunge φ1=0, Φ=φ°, φ2=0 fixtures make the c-axis angular distance between any two features exactly \|Φ_A − Φ_B\| (folded into [0°, 90°]); expected groupings derive in closed form from the chosen tolerance and neighbor adjacency. | -| Code paths enumerated | 9 of 9 (line-by-line walk of `GroupMicroTextureRegions::operator()`, `execute`, `getSeed`, `determineGrouping`, `randomizeParentIds`) | +| Code paths enumerated | **7 of 9 exercised** (9 enumerated by a line-by-line walk of `GroupMicroTextureRegions::operator()`, `execute`, `getSeed`, `determineGrouping`, `randomizeParentIds`). The two uncovered paths (#3 null-list guard, #6 background-phase skip) are defensive branches, listed with reasons in the Code path coverage table. | | Tests today | 5 test cases: Class 1 "Pure-Phi Bunge" (5 features, 3 groups), Class 4 "RandomizeParentIds invariants" (equivalence-class preservation + seed determinism), Class 1 "Tolerance Boundary" (3 features, 2 groups), defect-A regression pin (UseNonContiguousNeighbors=false runs without error), SIMPL 6.4+6.5 backwards-compat (DYNAMIC_SECTION). | | Exemplar archive | **None — data inlined in test source** (`test/GroupMicroTextureRegionsTest.cpp` namespace `AnalyticalFixtures`). Scaffold helper builds an `{nX,1,1}` ImageGeom + Cell/Feature/Ensemble AMs; per-feature pure-Phi quats and per-feature neighbor lists are set explicitly per test. | -| Legacy comparison | **Not run.** Legacy 6.5.171 randomizes parent IDs by default with a non-reproducible seed; bit-identical comparison is not meaningful. A 2025-10-23 fix applied to a local build of the legacy source exposes `RandomizeParentIds` as a user parameter (default false) — the same design SIMPLNX now uses post-fix; this V&V verifies SIMPLNX against an independent oracle. | +| Legacy comparison | **N/A — no public legacy equivalent.** In DREAM3D 6.5.171 `GroupMicroTextureRegions` is a **private** filter: it is listed in `_PrivateFilters` in `Source/Plugins/Reconstruction/ReconstructionFilters/SourceList.cmake` and registered via `ADD_SIMPL_FILTER(... FALSE ...)`, which the legacy source itself documents as "available from other filters but the user will not be able to use them from the DREAM3D user interface." It is therefore compiled but not instantiable from a pipeline, so no A/B run against 6.5.171 is possible. A fix applied to a local build of the legacy source promotes it to public and exposes `RandomizeParentIds`; that patched build is the corroborating reference for D1–D3. Secondary obstacle: legacy randomizes parent IDs from a clock-derived seed, so even once exposed it yields permutation-equivalent, never bit-identical, arrays. | | Bug flags | **None remaining.** Two port-time regressions (D1, D2) were found and fixed during this V&V cycle and are pinned by tests. One legacy 6.5.171 bug (D3) was already corrected in the original 2024 SIMPLNX port and is documented for migration users. | -| V&V phase | Phases 1, 3, 4, 5, 6, 7, 8, 9, 11 — complete. **Outstanding:** OOC build verification (dual-build protocol), Phase 13 (Status promotion to READY FOR REVIEW), second-engineer review of the oracle design and the patched-legacy reference fix. | +| V&V phase | Phases 1, 3, 4, 5, 6, 7, 8, 9, 11 — complete. Legacy comparison closed as **N/A** (private filter in 6.5.171 — see the Legacy comparison row). OOC verification **out of scope for this round** — not required by the MTR SBIR deliverable and owned separately by another engineer; additionally GMTR is serial with no OOC dispatch variant and no `ForceOutOfCore` sentinel in its tests, so an OOC build would exercise in-memory stores and prove nothing. Store-agnosticism is established by inspection instead (see Test inventory). **Outstanding:** second-engineer review of the oracle design — specifically the chain-transitive grouping characteristic (see Oracle § Second-engineer review). | ## Summary @@ -94,11 +94,15 @@ Companion assertions, applicable independent of which feature is picked as the f ### Second-engineer review -*Pending.* The c-axis closed-form derivation for pure-Phi Bunge angles is sibling-shared with `ComputeFeatureNeighborCAxisMisalignmentsFilter`, whose Class 1 oracle was reviewed previously; the same derivation applies here. A second-engineer pass on (a) the patched-legacy reference fix as the corroborating source for the D2 design, and (b) the on-the-boundary 12°-rejection assumption in Fixture B (i.e., that under `UseRunningAverage=false` the algorithm compares each candidate against `firstFeature`'s c-axis, not against the running seed's c-axis), is recommended before Status promotion. +*Pending.* The c-axis closed-form derivation for pure-Phi Bunge angles is sibling-shared with `ComputeFeatureNeighborCAxisMisalignmentsFilter`, whose Class 1 oracle was reviewed previously; the same derivation applies here. + +The specific item a second engineer should confirm is the **chain-transitive grouping characteristic** under `UseRunningAverage=false`. In `execute()` the BFS iterates `for(j = 0; j < groupList.size(); j++)` over a list that *grows* as neighbors are accepted, and passes `groupList[j]` to `determineGrouping()` as `referenceFeature`. The local variable is named `firstFeature`, but it is the **current frontier feature, not the seed** — so each candidate is compared against its immediate BFS predecessor, not against the seed and not against a running average. Grouping is therefore the transitive closure of the pairwise-tolerance relation along neighbor chains: with a 10° tolerance, features at Φ = 0°, 8°, 16° would all merge into one group even though the end members are 16° apart. **Is that the intended semantic?** It matches legacy `GroupFeatures::execute`, but it is a behaviour users should understand, and it is the one place where a wrong oracle would silently confirm a wrong implementation. + +Fixture B (`Class 1 Analytical (Tolerance Boundary)`; Φ = 0°/8°/20°, tol 10°) was hand-checked against **all three possible seed orders** — `getSeed()` picks randomly among unparented features, so the fixture must not depend on which feature seeds first. Seeding at F1, F2, or F3 all yield the same partition {F1,F2}, {F3}, because the only bridging edge (F2↔F3, 12°) exceeds tolerance from either direction. The assertion is therefore seed-order independent. ## Code path coverage -*9 of 9 paths exercised.* +*7 of 9 paths exercised.* Paths #3 and #6 are defensive/low-value branches and are listed below with their reasons; they are counted as uncovered rather than folded into the total. Source: `src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/GroupMicroTextureRegions.cpp` (266 lines). @@ -127,7 +131,9 @@ Logical phases: **(a) per-call init** in `operator()` (RNG seed, parent-ID init) | `OrientationAnalysis::GroupMicroTextureRegionsFilter: SIMPL Backwards Compatibility` | kept | DYNAMIC_SECTION over SIMPL 6.4 + 6.5 conversion fixtures. Validates UUID and argument-key conversion only; not a behavioral test. Was the only passing test under the old `[.][UNIMPLEMENTED][!mayfail]` regime. | | *(retired)* `OrientationAnalysis::GroupMicroTextureRegionsFilter: Valid Filter Execution` (tag `[.][UNIMPLEMENTED][!mayfail]`) | retired | Old test used empty `DataPath{}` arguments throughout; could not pass preflight and was tagged hidden/expected-fail. Replaced by the two `Class 1 Analytical` tests above. The replacement actually exercises the algorithm with real data. | -All non-retired tests pass on `vv/group_microtexture_regions` (verified on the in-core release build at 2026-06-11). OOC verification: pending — this V&V cycle did not run the OOC build profile, but the algorithm reads `Int32Array`, `Float32Array`, `UInt32Array`, and `NeighborList` via reference-binding in the constructor; OOC-incompatible patterns (raw-pointer access, parallel writes to the same array) are not used. +All 5 non-retired tests pass in the in-core build — **re-verified 2026-08-10** on preset `NX-Com-Qt69-Vtk96-Rel` after a full rebuild (`ctest -R GroupMicroTexture` → 5/5 passed, 0 failed, 0.15 s). This supersedes the earlier 2026-06-11 spot-check, which predated several upstream merges into `develop`. + +**OOC: not run, by design.** `GroupMicroTextureRegions` is a serial algorithm with no out-of-core dispatch variant (no `DispatchAlgorithm` split, no Scanline implementation, no bulk `copyIntoBuffer`/`copyFromBuffer` path), so `SIMPLNX_TEST_ALGORITHM_PATH` has nothing to switch for it. Its tests also carry no `UnitTest::PreferencesSentinel(DataStorageMode::ForceOutOfCore, ...)`, and sentinel-less tests remain in-memory even inside an OOC build — so running the existing suite under an OOC configuration would exercise in-memory stores and prove nothing. Store-agnosticism is therefore established **by inspection**: the algorithm binds `Int32Array&`, `Float32Array&`, `UInt32Array&`, and `NeighborList&` and reads them only through the public `AbstractDataStore` API (`getValue`/`setValue`/`operator[]`); it takes no raw pointers into store memory and performs no parallel writes, the two patterns that break under a disk-backed store. Note that `IParallelAlgorithm::requireArraysInMemory()` is **not** applicable here — it only toggles parallelization off when arrays are OOC-backed, and this algorithm is serial. **Performance caveat (not a correctness gap):** the grouping BFS walks neighbor lists in random feature order, which is chunk-cache hostile; a disk-backed run on a large feature set would be slow even though it would be correct. **Out of scope for this V&V round, by decision (2026-08-10):** OOC functionality is not required by the MTR SBIR deliverable, and out-of-core support across the filter set is owned separately by another engineer. Closing the gate would mean adding a `ForceOutOfCore` sentinel fixture plus an OOC build configuration; that work is deliberately excluded here rather than left undone by oversight, and it does not gate this report. ## Exemplar archive @@ -139,8 +145,8 @@ The fixture scaffold (`AnalyticalFixtures::CreateScaffold(numFeatures)`) builds ## Deviations from DREAM3D 6.5.171 -Direct array-by-array comparison against 6.5.171 is not meaningful: 6.5.171 randomizes parent IDs by default using a clock-derived seed (irreproducible) and produces *grouping equivalence classes* that match SIMPLNX's groups under any permutation, but never *bit-identical* arrays. The three documented deviations are design-level statements rather than per-array diffs. +Direct array-by-array comparison against 6.5.171 is **not possible**, for a structural reason: `GroupMicroTextureRegions` ships in 6.5.171 as a **private** filter (`_PrivateFilters` in `Source/Plugins/Reconstruction/ReconstructionFilters/SourceList.cmake`, registered `ADD_SIMPL_FILTER(... FALSE ...)`). Private filters are compiled but, in the legacy source's own words, "the user will not be able to use them from the DREAM3D user interface" — they are not registered for pipeline use, so `PipelineRunner` cannot instantiate the filter and no A/B fixture can be run. Even setting that aside, 6.5.171 randomizes parent IDs from a clock-derived seed, so it yields grouping equivalence classes that match SIMPLNX under permutation but never bit-identical arrays. The three deviations below are therefore design-level statements derived from source inspection and from a local build of the legacy source with the filter promoted to public — not per-array diffs. - `GroupMicroTextureRegionsFilter-D1` — Defect A: pre-fix SIMPLNX returned error `-99345` when `UseNonContiguousNeighbors=false` (filter unusable in default mode). **Fixed.** See `vv/deviations/GroupMicroTextureRegionsFilter.md`. - `GroupMicroTextureRegionsFilter-D2` — Defect B: SIMPLNX never randomized parent IDs (legacy 6.5.171 always randomized; a fix applied to a local build of the legacy source exposes `RandomizeParentIds` as a user parameter, default false). **Fixed by exposing `RandomizeParentIds` (default false) + restoring the helper and plumbing the user seed through.** See `vv/deviations/GroupMicroTextureRegionsFilter.md`. -- `GroupMicroTextureRegionsFilter-D3` — Legacy 6.5.171 bug: when `UseRunningAverage=true`, `phase1` is never assigned and the Hex_High acceptance check silently fails — no features ever group. Bug introduced upstream by J. Tucker on 2014-01-30 (commit `7e49e52f` in original DREAM3D). SIMPLNX corrected this in the 2024-01-08 initial port; the same fix was applied to a local build of the legacy source. **Documented for migration users.** See `vv/deviations/GroupMicroTextureRegionsFilter.md`. +- `GroupMicroTextureRegionsFilter-D3` — Legacy 6.5.171 bug: when `UseRunningAverage=true`, `phase1` is never assigned and the Hex_High acceptance check silently fails — no features ever group. Bug introduced upstream by J. Tucker on 2014-01-30 (commit `7e49e52f` in original DREAM3D). SIMPLNX corrected this in the 2024-01-08 initial port; the same fix was applied to a local build of the legacy source. **Note:** because the filter was private in 6.5.171, this bug was not reachable by 6.5.171 end users — it is documented for provenance and for anyone who built the legacy filter themselves, not as a migration hazard. See `vv/deviations/GroupMicroTextureRegionsFilter.md`. diff --git a/src/Plugins/OrientationAnalysis/vv/deviations/GroupMicroTextureRegionsFilter.md b/src/Plugins/OrientationAnalysis/vv/deviations/GroupMicroTextureRegionsFilter.md index e34acf185f..cbb5f8665d 100644 --- a/src/Plugins/OrientationAnalysis/vv/deviations/GroupMicroTextureRegionsFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/deviations/GroupMicroTextureRegionsFilter.md @@ -56,7 +56,7 @@ Legacy 6.5.171 `GroupFeatures::execute()` gates the *use* of `nonContigNeighList **Fix:** Adopted the patched-legacy design (the 2025-10-23 fix applied to a local build of the legacy source): added a new user parameter `RandomizeParentIds` (default `false`, so default behaviour is reproducible parent-id assignment). Restored a `randomizeParentIds(totalPoints, totalParentIds)` helper that performs a Fisher-Yates shuffle using `m_Generator` already seeded by `operator()`. Fixed the `operator()` RNG initialization to use `m_InputValues->SeedValue`. `parametersVersion()` bumped from 1 to 2 to flag the new parameter for SIMPL-conversion JSON. -**Affected users:** Anyone migrating a pipeline from 6.5.171 that depended on randomized parent IDs (e.g., feeding the parent IDs straight into a color-mapped visualization where adjacent groups should not share the same color by accident). Post-fix: +**Affected users:** *No 6.5.171 end user, strictly speaking* — the filter was private in 6.5.171 (see D3) and could not be run from a pipeline, so there is no population of users migrating a working 6.5.171 GMTR pipeline. The deviation matters instead as a **behavioural difference between pre-fix and post-fix SIMPLNX**, and to anyone comparing against a local build of the legacy source with the filter promoted to public: parent IDs that were previously always sequential are now optionally shuffled, which changes any downstream consumer that treats the parent ID as meaningful rather than as an opaque label (e.g., feeding parent IDs straight into a color-mapped visualization where adjacent groups should not share a color by accident). Post-fix: - `RandomizeParentIds=false` (default) → reproducible parent IDs, suitable for diff testing and exemplar comparisons. - `RandomizeParentIds=true, UseSeed=false` → 6.5.171-like behaviour (clock-derived seed, irreproducible). - `RandomizeParentIds=true, UseSeed=true, SeedValue=` → reproducible randomization for users who want both shuffled IDs *and* run-to-run reproducibility. @@ -97,6 +97,6 @@ When `UseRunningAverage=true`, `phase1` stays at 0; the subsequent `phase1 == He **Fix in SIMPLNX:** The 2024-01-08 initial port (`ca6d0aa`) corrected the bug by assigning `phase1` outside the conditional, before the Hex_High check. The same fix was *deliberately applied to a local build of the legacy source* (2025-10-23) — confirmed by inspecting the pre-fix legacy source, which still has the buggy `phase1` declaration. The legacy-side fix renames the variable to `phase1Xtal`, hoists the assignment out of the `if(!m_UseRunningAverage)` block, and ships with a developer comment that explicitly names the J. Tucker 2014-01-30 introduction as the bug source. The 6.5.171 release line was never patched. -**Affected users:** Anyone running 6.5.171 with `UseRunningAverage=true` saw degenerate output (one group per feature) without warning. Users migrating that workflow to SIMPLNX will see *real* groupings for the first time, and downstream filters that consumed the degenerate output may behave differently. +**Affected users:** *Effectively none in the released product.* `GroupMicroTextureRegions` was a **private** filter in 6.5.171 (`_PrivateFilters` in `Source/Plugins/Reconstruction/ReconstructionFilters/SourceList.cmake`) — compiled but not registered for user or pipeline access — so no 6.5.171 end user could reach this code path. The impact is limited to anyone who built the legacy source with the filter promoted to public: they would have seen degenerate output (one group per feature) with `UseRunningAverage=true`, without warning. This entry is retained for provenance and to document why the bug survived from 2014 to discovery: it lived in a filter nobody could run. **Recommendation:** Trust SIMPLNX. The 6.5.171 result with `UseRunningAverage=true` was mathematically incorrect; SIMPLNX produces the result the filter has always claimed to produce. diff --git a/src/Plugins/OrientationAnalysis/vv/provenance/GroupMicroTextureRegionsFilter.md b/src/Plugins/OrientationAnalysis/vv/provenance/GroupMicroTextureRegionsFilter.md index 4d59b9aaef..06ce4f261f 100644 --- a/src/Plugins/OrientationAnalysis/vv/provenance/GroupMicroTextureRegionsFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/provenance/GroupMicroTextureRegionsFilter.md @@ -66,7 +66,21 @@ N/A — Class 1 (Analytical) + Class 4 (Invariant) oracle. Both lower-drift clas ## Second-engineer oracle review -- **Reviewer:** *Pending — the pure-Phi Bunge derivation is sibling-shared with `ComputeFeatureNeighborCAxisMisalignmentsFilter` (already reviewed); a second engineer should still confirm (a) the F2↔F3 rejection at 12° in the Tolerance Boundary fixture is correct under `UseRunningAverage=false` semantics (i.e., that the algorithm compares each BFS step against the immediately-preceding feature's c-axis, not against the original seed's c-axis), and (b) the invariant list in `RandomizeParentIds invariants` is exhaustive — that no observable behaviour of the shuffle is left unasserted.* +- **Reviewer:** *Pending.* The pure-Phi Bunge derivation is sibling-shared with `ComputeFeatureNeighborCAxisMisalignmentsFilter` (already reviewed). + + **Resolved by source inspection during the 2026-08-10 gate-closing pass (V&V author; still needs independent confirmation):** item (a) below — the `UseRunningAverage=false` + comparison target — is confirmed correct as stated in this sidecar. `execute()` iterates `for(j = 0; j < groupList.size(); j++)` over a list that grows as neighbors are + accepted and passes `groupList[j]` to `determineGrouping()` as `referenceFeature`; the local is misleadingly named `firstFeature` but is the current BFS frontier feature, so + each candidate is compared against its immediately-preceding feature, not the seed and not a running average. The Tolerance Boundary fixture was additionally re-derived under + **all three possible seed orders** (`getSeed()` draws randomly among unparented features): seeding at F1, F2, or F3 each yields the partition {F1,F2}, {F3}, so the assertion + is seed-order independent. Note the report body previously described this semantic incorrectly and the test comment was self-contradictory; both were corrected to match this + sidecar, which was right. + + **Still open for a second engineer:** + (a) *Intent, not mechanics* — the comparison rule makes grouping the **transitive closure** of the pairwise-tolerance relation along neighbor chains: at 10° tolerance, + features at Φ = 0°, 8°, 16° all merge into one group despite the end members being 16° apart. This matches legacy `GroupFeatures::execute`, but should be confirmed as + the intended user-facing semantic rather than an inherited accident. + (b) Whether the invariant list in `RandomizeParentIds invariants` is exhaustive — that no observable behaviour of the shuffle is left unasserted. - **Date:** *YYYY-MM-DD (pending)* - **Skip reason** (if skipped): *N/A — review recommended.*