From d636244f6f417dcd6006971136c45d1178a52fd9 Mon Sep 17 00:00:00 2001 From: menes97 <38889017+menes97@users.noreply.github.com> Date: Thu, 12 Nov 2026 22:22:23 +0300 Subject: [PATCH] Diff: match each baseline element at most once 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 --- src/NwcIO/Model/Diff/NwcDiffEngine.cs | 32 ++++++++++--- tests/NwcIO.Tests/DiffEngineTests.cs | 68 +++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/NwcIO/Model/Diff/NwcDiffEngine.cs b/src/NwcIO/Model/Diff/NwcDiffEngine.cs index 519451c..c84a82c 100644 --- a/src/NwcIO/Model/Diff/NwcDiffEngine.cs +++ b/src/NwcIO/Model/Diff/NwcDiffEngine.cs @@ -68,19 +68,21 @@ private static void PerformDiff( List revItems, NwcDiffOptions options) { - // 1. Build lookup tables: by Guid first, then by composite MatchKey - var baseByGuid = new Dictionary(); - var baseByKey = new Dictionary(StringComparer.OrdinalIgnoreCase); + // 1. Build lookup tables: by Guid first, then by composite MatchKey. Several baseline items can share a GUID + // (or, across the models of an assembly, a key), so each entry queues them in order and a match takes the + // first one: every baseline item is matched at most once. + var baseByGuid = new Dictionary>(); + var baseByKey = new Dictionary>(StringComparer.OrdinalIgnoreCase); foreach (var item in baseItems) { if (item.ElementGuid.HasValue && item.ElementGuid.Value != Guid.Empty) { - baseByGuid[item.ElementGuid.Value] = item; + Enqueue(baseByGuid, item.ElementGuid.Value, item); } else { - baseByKey[item.MatchKey] = item; + Enqueue(baseByKey, item.MatchKey, item); } } @@ -97,11 +99,11 @@ private static void PerformDiff( DiffItem? baseItem = null; if (rev.ElementGuid.HasValue && rev.ElementGuid.Value != Guid.Empty) { - baseByGuid.TryGetValue(rev.ElementGuid.Value, out baseItem); + baseItem = Dequeue(baseByGuid, rev.ElementGuid.Value); } if (baseItem is null && !string.IsNullOrEmpty(rev.MatchKey)) { - baseByKey.TryGetValue(rev.MatchKey, out baseItem); + baseItem = Dequeue(baseByKey, rev.MatchKey); } if (baseItem is null) @@ -221,6 +223,22 @@ private static void PerformDiff( } } + private static void Enqueue(Dictionary> lookup, TKey key, DiffItem item) + where TKey : notnull + { + if (!lookup.TryGetValue(key, out var queue)) + { + queue = new Queue(); + lookup[key] = queue; + } + + queue.Enqueue(item); + } + + private static DiffItem? Dequeue(Dictionary> lookup, TKey key) + where TKey : notnull => + lookup.TryGetValue(key, out var queue) && queue.TryDequeue(out var item) ? item : null; + private static bool PassesFilter(DiffItem item, string? filter) { if (string.IsNullOrWhiteSpace(filter)) return true; diff --git a/tests/NwcIO.Tests/DiffEngineTests.cs b/tests/NwcIO.Tests/DiffEngineTests.cs index d576a85..10c6c3d 100644 --- a/tests/NwcIO.Tests/DiffEngineTests.cs +++ b/tests/NwcIO.Tests/DiffEngineTests.cs @@ -244,4 +244,72 @@ public void FederatedAssembly_CompareProducesReport() Assert.Equal("MeshNew", report.Added[0].Name); Assert.Single(report.Unchanged); } + + private static NwcDocument CreateSharedGuidDocument(Guid guid, int count) + { + var root = new NwcNode { Id = 1, Name = "Root", Class = "Root" }; + for (int i = 0; i < count; i++) + { + root.Children.Add(new NwcNode { Id = 2 + i, Name = "Part", Class = "LcRevitData_Element", ElementGuid = guid }); + } + + return NwcDocument.Create(rootNode: root); + } + + [Fact] + public void Compare_DuplicateGuidsInDocument_MatchesEachElementOnce() + { + var guid = Guid.NewGuid(); + + var report = CreateSharedGuidDocument(guid, 3).Compare(CreateSharedGuidDocument(guid, 3)); + + Assert.Empty(report.Added); + Assert.Empty(report.Deleted); + Assert.Empty(report.Modified); + Assert.Equal(3, report.Unchanged.Count); + Assert.Equal([2, 3, 4], report.Unchanged.Select(d => d.BaselineNode!.Id).Order()); + } + + [Fact] + public void Compare_DuplicateGuidRemovedFromRevision_ReportsOneDeletion() + { + var guid = Guid.NewGuid(); + + var report = CreateSharedGuidDocument(guid, 3).Compare(CreateSharedGuidDocument(guid, 2)); + + Assert.Empty(report.Added); + Assert.Single(report.Deleted); + Assert.Equal(2, report.Unchanged.Count); + } + + [Fact] + public void Compare_GuidSharedAcrossAssemblyModels_MatchesEachElementOnce() + { + var guid = Guid.NewGuid(); + var baseline = NwcAssembly.Create("A", CreateSharedGuidDocument(guid, 1), CreateSharedGuidDocument(guid, 1)); + var revision = NwcAssembly.Create("B", CreateSharedGuidDocument(guid, 1), CreateSharedGuidDocument(guid, 1)); + + var report = baseline.Compare(revision); + + Assert.Empty(report.Added); + Assert.Empty(report.Deleted); + Assert.Equal(2, report.Unchanged.Count); + } + + [Fact] + public void Compare_SameMeshNameAcrossAssemblyModels_MatchesEachElementOnce() + { + var baseline = NwcAssembly.Create("A", + NwcDocument.Create(meshes: [CreateBoxMesh(1, "Wall", new Vector3d(0, 0, 0))]), + NwcDocument.Create(meshes: [CreateBoxMesh(1, "Wall", new Vector3d(0, 0, 0))])); + var revision = NwcAssembly.Create("B", + NwcDocument.Create(meshes: [CreateBoxMesh(1, "Wall", new Vector3d(0, 0, 0))]), + NwcDocument.Create(meshes: [CreateBoxMesh(1, "Wall", new Vector3d(0, 0, 0))])); + + var report = baseline.Compare(revision); + + Assert.Empty(report.Added); + Assert.Empty(report.Deleted); + Assert.Equal(2, report.Unchanged.Count); + } }