Repository navigation
Conversation
The GUID lookup kept only the last baseline item of each GUID, so every revision item with that GUID matched the same one and the others were reported as Deleted, even for identical inputs. The key lookup had the same collision across the models of an assembly. Both lookups now queue the baseline items in order and a match takes the next one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mahach666
left a comment
mahach666
left a comment
There was a problem hiding this comment.
Changes requested: the intended fix is useful, but the inline comment below describes a regression reproduced against the reviewed head and the unchanged base. Please address it and add the indicated regression test before merging.
Finding: Disambiguate matches by the contributing model.
| if (baseItem is null && !string.IsNullOrEmpty(rev.MatchKey)) | ||
| { | ||
| baseByKey.TryGetValue(rev.MatchKey, out baseItem); | ||
| baseItem = Dequeue(baseByKey, rev.MatchKey); |
There was a problem hiding this comment.
[P2] Disambiguate matches by the contributing model
The shared FIFO queue can now match an unchanged element to a different source model.
Reproduction: baseline assembly [A.rvt, B.rvt], revision assembly [B.rvt]. Each document contains one GUID-less mesh named Wall; A's mesh is at X=0 and B's is at X=100. Both acquire the same MatchKey. Main/base correctly reports B as Unchanged and A as Deleted. This PR dequeues A for B, reports it as Modified, and reports the unchanged B mesh as Deleted. Reordering [A,B] to [B,A] also produces two false modifications.
Please retain the contributing model's identity when resolving duplicate match keys (and use it to disambiguate duplicate GUID candidates where appropriate), rather than relying on assembly collection order. Add tests for removing the first model and reordering unchanged models.
Fixes #62
NwcDiffEngine.PerformDiffkept only the last baseline item of each GUID, so every revision item with that GUID matched the same baseline item. The others were reported asDeletedeven for identical inputs. TheMatchKeylookup had the same problem across the models of an assembly, because each model numbers its own keys from 0: two models with a mesh of the same name gave a deletion too.Both lookups now keep a queue of the baseline items for each GUID or key, in collection order, and a match takes the next one. So each baseline item is matched at most once, and items that share a GUID pair up in order (in an assembly, model by model). I used this instead of a model qualifier on the keys because it also covers GUIDs shared across models, and it keeps matching by GUID independent of where an element sits. Unique GUIDs and keys match as before.
Tests (
DiffEngineTests):Compare_DuplicateGuidsInDocument_MatchesEachElementOnce: three nodes sharing a GUID, compared with an identical copy, are 3Unchanged(each baseline node once) with nothing added, deleted or modified.Compare_DuplicateGuidRemovedFromRevision_ReportsOneDeletion: three against two gives oneDeleted.Compare_GuidSharedAcrossAssemblyModels_MatchesEachElementOnce: the assembly case from the issue.Compare_SameMeshNameAcrossAssemblyModels_MatchesEachElementOnce: two models with a mesh namedWall.All four fail without the change.
dotnet build NwcIO.slnxsucceeds, andNwcIO.Testspasses locally: 243 passed, and 2 skipped because they need private Navisworks files (.NET 10.0.400 on Windows, withglobal.jsonpinned down to it temporarily; not committed).🤖 Generated with Claude Code