Merge branch 'main' into issue-65-5d06345e60dc

konard · claude · konard · commit 9b3b9e04f8cc · 2026-01-08T05:40:02.000+01:00
Resolved conflict in rust/src/changes_simplifier.rs: - Kept new remove_duplicate_before_states function from main (Issue #26 fix) - Moved inline tests to separate test file per reviewer feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
diff --git a/csharp/Foundation.Data.Doublets.Cli.Tests/ChangesSimplifier.cs b/csharp/Foundation.Data.Doublets.Cli.Tests/ChangesSimplifier.cs
@@ -327,6 +327,77 @@ public void SimplifyChanges_SpecificExample_KeepsUnchangedStates()
       AssertChangeSetEqual(expectedSimplifiedChanges, simplifiedChanges);
     }
 
+    [Fact]
+    public void SimplifyChanges_Issue26_UpdateOperationSimplification()
+    {
+      // Arrange - This represents the scenario described in GitHub issue #26
+      // Where links (1: 1 2) and (2: 2 1) are being updated to swap source and target
+      // The issue was that intermediate steps were being shown instead of the final transformation
+      var changes = new List<(Link<uint> Before, Link<uint> After)>
+      {
+        // Step 1: Link (1: 1 2) is first deleted (becomes null/empty)
+        (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 0, source: 0, target: 0)),
+        
+        // Step 2: New link (1: 2 1) is created (swapped source and target)
+        (new Link<uint>(index: 0, source: 0, target: 0), new Link<uint>(index: 1, source: 2, target: 1)),
+        
+        // Step 3: Link (2: 2 1) is directly updated to (2: 1 2)
+        (new Link<uint>(index: 2, source: 2, target: 1), new Link<uint>(index: 2, source: 1, target: 2)),
+      };
+
+      // Expected - The simplification should show only the initial-to-final transformations
+      var expectedSimplifiedChanges = new List<(Link<uint> Before, Link<uint> After)>
+      {
+        (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 1, source: 2, target: 1)),
+        (new Link<uint>(index: 2, source: 2, target: 1), new Link<uint>(index: 2, source: 1, target: 2)),
+      };
+
+      // Act
+      var simplifiedChanges = SimplifyChanges(changes).ToList();
+
+      // Assert
+      AssertChangeSetEqual(expectedSimplifiedChanges, simplifiedChanges);
+    }
+
+    [Fact]
+    public void SimplifyChanges_Issue26_AlternativeScenario_NoSimplificationOccurs()
+    {
+      // Arrange - This tests a different scenario that might represent the actual issue
+      // Maybe the problem is that the changes are NOT being chained correctly
+      // Let's simulate what might happen if the simplifier doesn't work correctly
+      var changes = new List<(Link<uint> Before, Link<uint> After)>
+      {
+        // Let's say we get these individual changes that don't form a proper chain
+        (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 0, source: 0, target: 0)),  // delete
+        (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 1, source: 2, target: 1)),  // direct update (this might be what's reported)
+        (new Link<uint>(index: 2, source: 2, target: 1), new Link<uint>(index: 2, source: 1, target: 2)),  // direct update
+      };
+
+      // Act
+      var simplifiedChanges = SimplifyChanges(changes).ToList();
+
+      // Debug output
+      Console.WriteLine("=== Debug: Alternative Scenario ===");
+      Console.WriteLine("Input changes:");
+      for (int i = 0; i < changes.Count; i++)
+      {
+        var (b, a) = changes[i];
+        Console.WriteLine($"  {i + 1}. ({b.Index}: {b.Source} {b.Target}) -> ({a.Index}: {a.Source} {a.Target})");
+      }
+
+      Console.WriteLine("Actual simplified changes:");
+      for (int i = 0; i < simplifiedChanges.Count; i++)
+      {
+        var (b, a) = simplifiedChanges[i];
+        Console.WriteLine($"  {i + 1}. ({b.Index}: {b.Source} {b.Target}) -> ({a.Index}: {a.Source} {a.Target})");
+      }
+      Console.WriteLine($"Count: {simplifiedChanges.Count}");
+      Console.WriteLine("=== End Debug ===");
+
+      // The issue might be that we get 3 changes instead of 2
+      // If the simplifier doesn't work, we'd see all 3 changes
+    }
+
     private static void AssertChangeSetEqual(
       List<(Link<uint> Before, Link<uint> After)> expectedSimplifiedChanges,
       List<(Link<uint> Before, Link<uint> After)> simplifiedChanges
diff --git a/csharp/Foundation.Data.Doublets.Cli/ChangesSimplifier.cs b/csharp/Foundation.Data.Doublets.Cli/ChangesSimplifier.cs
@@ -27,6 +27,10 @@ public static class ChangesSimplifier
         return Enumerable.Empty<(Link<uint>, Link<uint>)>();
       }
 
+      // **FIX for Issue #26**: Remove duplicate before states by keeping the last occurrence
+      // This handles cases where the same link is reported with multiple different transformations
+      changesList = RemoveDuplicateBeforeStates(changesList);
+
       // First, handle unchanged states directly
       var unchangedStates = new List<(Link<uint> Before, Link<uint> After)>();
       var changedStates = new List<(Link<uint> Before, Link<uint> After)>();
@@ -124,6 +128,61 @@ public static class ChangesSimplifier
           .ThenBy(r => r.After.Target);
     }
 
+    /// <summary>
+    /// Removes problematic duplicate before states that lead to simplification issues.
+    /// This fixes Issue #26 where multiple transformations from the same before state
+    /// to conflicting after states (including null states) would cause the simplifier to fail.
+    /// 
+    /// The key insight: If we have multiple transitions from the same before state,
+    /// and one of them is to a "null" state (0: 0 0), we should prefer the non-null transition
+    /// as it represents the actual final transformation.
+    /// </summary>
+    /// <param name="changes">The list of changes that may contain problematic duplicate before states</param>
+    /// <returns>A list with problematic duplicates resolved</returns>
+    private static List<(Link<uint> Before, Link<uint> After)> RemoveDuplicateBeforeStates(
+        List<(Link<uint> Before, Link<uint> After)> changes)
+    {
+      // Group changes by their before state
+      var groupedChanges = changes.GroupBy(c => c.Before, LinkEqualityComparer.Instance);
+      
+      var result = new List<(Link<uint> Before, Link<uint> After)>();
+      
+      foreach (var group in groupedChanges)
+      {
+        var changesForThisBefore = group.ToList();
+        
+        if (changesForThisBefore.Count == 1)
+        {
+          // No duplicates, keep as is
+          result.AddRange(changesForThisBefore);
+        }
+        else
+        {
+          // Multiple changes from the same before state
+          // Check if any of them is to a null state (0: 0 0)
+          var nullTransition = changesForThisBefore.FirstOrDefault(c => 
+              c.After.Index == 0 && c.After.Source == 0 && c.After.Target == 0);
+          var nonNullTransitions = changesForThisBefore.Where(c => 
+              !(c.After.Index == 0 && c.After.Source == 0 && c.After.Target == 0)).ToList();
+          
+          if (nullTransition != default && nonNullTransitions.Count > 0)
+          {
+            // Issue #26 scenario: We have both null and non-null transitions
+            // Prefer the non-null transitions as they represent the actual final states
+            result.AddRange(nonNullTransitions);
+          }
+          else
+          {
+            // No null transitions involved, this is a legitimate multiple-branch scenario
+            // Keep all transitions
+            result.AddRange(changesForThisBefore);
+          }
+        }
+      }
+      
+      return result;
+    }
+
     /// <summary>
     /// An equality comparer for Link<uint> that checks Index/Source/Target.
     /// </summary>
diff --git a/csharp/Foundation.Data.Doublets.Cli/Program.cs b/csharp/Foundation.Data.Doublets.Cli/Program.cs
@@ -131,9 +131,27 @@
 
     if (changes && changesList.Any())
     {
+      // Debug: Print raw changes before simplification (if trace is enabled)
+      if (trace)
+      {
+        Console.WriteLine("[DEBUG] Raw changes before simplification:");
+        for (int i = 0; i < changesList.Count; i++)
+        {
+          var (beforeLink, afterLink) = changesList[i];
+          Console.WriteLine($"[DEBUG] {i + 1}. ({beforeLink.Index}: {beforeLink.Source} {beforeLink.Target}) -> ({afterLink.Index}: {afterLink.Source} {afterLink.Target})");
+        }
+        Console.WriteLine($"[DEBUG] Total raw changes: {changesList.Count}");
+      }
+
       // Simplify the collected changes
       var simplifiedChanges = SimplifyChanges(changesList);
 
+      // Debug: Print simplified changes count (if trace is enabled)
+      if (trace)
+      {
+        Console.WriteLine($"[DEBUG] Simplified changes count: {simplifiedChanges.Count()}");
+      }
+
       // Print the simplified changes
       foreach (var (linkBefore, linkAfter) in simplifiedChanges)
       {
diff --git a/examples/debug_changes.cs b/examples/debug_changes.cs
@@ -0,0 +1,27 @@
+using Platform.Data.Doublets;
+using Foundation.Data.Doublets.Cli;
+using static Foundation.Data.Doublets.Cli.ChangesSimplifier;
+
+// Test the problematic scenario from the issue
+var changes = new List<(Link<uint> Before, Link<uint> After)>
+{
+    // This simulates what might be happening in the update operation
+    // Let's say we have links being swapped: (1:1 2) -> (1:2 1) and (2:2 1) -> (2:1 2)
+    
+    // From the issue it looks like these might be the intermediate steps:
+    (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 1, source: 2, target: 1)),
+    (new Link<uint>(index: 2, source: 2, target: 1), new Link<uint>(index: 2, source: 1, target: 2)),
+};
+
+Console.WriteLine("Original changes:");
+foreach (var (before, after) in changes)
+{
+    Console.WriteLine($"({before.Index}: {before.Source} {before.Target}) -> ({after.Index}: {after.Source} {after.Target})");
+}
+
+Console.WriteLine("\nSimplified changes:");
+var simplified = SimplifyChanges(changes).ToList();
+foreach (var (before, after) in simplified)
+{
+    Console.WriteLine($"({before.Index}: {before.Source} {before.Target}) -> ({after.Index}: {after.Source} {after.Target})");
+}
diff --git a/examples/test1_output.txt b/examples/test1_output.txt
diff --git a/examples/test_issue_scenario.cs b/examples/test_issue_scenario.cs
@@ -0,0 +1,49 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Platform.Data.Doublets;
+using Foundation.Data.Doublets.Cli;
+using static Foundation.Data.Doublets.Cli.ChangesSimplifier;
+
+// Simulate the exact scenario from the issue
+Console.WriteLine("=== Testing Issue #26 Scenario ===");
+
+// This simulates what might be happening based on the issue output:
+// ((1: 1 2)) ()           - Link (1: 1 2) is deleted  
+// ((1: 1 2)) ((1: 2 1))   - Link (1: 1 2) becomes (1: 2 1)
+// ((2: 2 1)) ((2: 1 2))   - Link (2: 2 1) becomes (2: 1 2)
+
+var changes = new List<(Link<uint> Before, Link<uint> After)>
+{
+    // First transformation: (1: 1 2) -> () (deletion)
+    (new Link<uint>(index: 1, source: 1, target: 2), new Link<uint>(index: 0, source: 0, target: 0)),
+    
+    // Second transformation: () -> (1: 2 1) (creation)
+    (new Link<uint>(index: 0, source: 0, target: 0), new Link<uint>(index: 1, source: 2, target: 1)),
+    
+    // Third transformation: (2: 2 1) -> (2: 1 2) (direct update)
+    (new Link<uint>(index: 2, source: 2, target: 1), new Link<uint>(index: 2, source: 1, target: 2)),
+};
+
+Console.WriteLine("Original changes (as they might occur in the system):");
+for (int i = 0; i < changes.Count; i++)
+{
+    var (before, after) = changes[i];
+    Console.WriteLine($"{i + 1}. ({before.Index}: {before.Source} {before.Target}) -> ({after.Index}: {after.Source} {after.Target})");
+}
+
+Console.WriteLine("\nSimplified changes (what should be shown):");
+var simplified = SimplifyChanges(changes).ToList();
+for (int i = 0; i < simplified.Count; i++)
+{
+    var (before, after) = simplified[i];
+    Console.WriteLine($"{i + 1}. ({before.Index}: {before.Source} {before.Target}) -> ({after.Index}: {after.Source} {after.Target})");
+}
+
+Console.WriteLine("\n=== Expected vs Actual ===");
+Console.WriteLine("Expected: Only the final transformations should be shown");
+Console.WriteLine("- (1: 1 2) -> (1: 2 1)");
+Console.WriteLine("- (2: 2 1) -> (2: 1 2)");
+
+Console.WriteLine($"\nActual count: {simplified.Count}");
+Console.WriteLine("If this count is more than 2, then the issue is reproduced.");
diff --git a/rust/src/changes_simplifier.rs b/rust/src/changes_simplifier.rs
@@ -16,6 +16,10 @@ pub fn simplify_changes(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
         return vec![];
     }
 
+    // **FIX for Issue #26**: Remove duplicate before states by keeping the non-null transitions
+    // This handles cases where the same link is reported with multiple different transformations
+    let changes = remove_duplicate_before_states(changes);
+
     // First, handle unchanged states directly
     let mut unchanged_states = Vec::new();
     let mut changed_states = Vec::new();
@@ -105,3 +109,51 @@ pub fn simplify_changes(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
 
     results
 }
+
+/// Removes problematic duplicate before states that lead to simplification issues.
+/// This fixes Issue #26 where multiple transformations from the same before state
+/// to conflicting after states (including null states) would cause the simplifier to fail.
+///
+/// The key insight: If we have multiple transitions from the same before state,
+/// and one of them is to a "null" state (0: 0 0), we should prefer the non-null transition
+/// as it represents the actual final transformation.
+fn remove_duplicate_before_states(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
+    // Group changes by their before state
+    let mut grouped: HashMap<Link, Vec<(Link, Link)>> = HashMap::new();
+    for change in changes {
+        grouped.entry(change.0).or_default().push(change);
+    }
+
+    let mut result = Vec::new();
+
+    for (_before, changes_for_this_before) in grouped {
+        if changes_for_this_before.len() == 1 {
+            // No duplicates, keep as is
+            result.extend(changes_for_this_before);
+        } else {
+            // Multiple changes from the same before state
+            // Check if any of them is to a null state (0: 0 0)
+            let null_link = Link::new(0, 0, 0);
+            let has_null_transition = changes_for_this_before
+                .iter()
+                .any(|(_, after)| *after == null_link);
+            let non_null_transitions: Vec<_> = changes_for_this_before
+                .iter()
+                .filter(|(_, after)| *after != null_link)
+                .cloned()
+                .collect();
+
+            if has_null_transition && !non_null_transitions.is_empty() {
+                // Issue #26 scenario: We have both null and non-null transitions
+                // Prefer the non-null transitions as they represent the actual final states
+                result.extend(non_null_transitions);
+            } else {
+                // No null transitions involved, this is a legitimate multiple-branch scenario
+                // Keep all transitions
+                result.extend(changes_for_this_before);
+            }
+        }
+    }
+
+    result
+}
diff --git a/rust/tests/changes_simplifier_tests.rs b/rust/tests/changes_simplifier_tests.rs
