diff --git a/csharp/Foundation.Data.Doublets.Cli.Tests/ChangesSimplifier.cs b/csharp/Foundation.Data.Doublets.Cli.Tests/ChangesSimplifier.cs index a1c3b57..5662198 100644 --- 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 Before, Link After)> + { + // Step 1: Link (1: 1 2) is first deleted (becomes null/empty) + (new Link(index: 1, source: 1, target: 2), new Link(index: 0, source: 0, target: 0)), + + // Step 2: New link (1: 2 1) is created (swapped source and target) + (new Link(index: 0, source: 0, target: 0), new Link(index: 1, source: 2, target: 1)), + + // Step 3: Link (2: 2 1) is directly updated to (2: 1 2) + (new Link(index: 2, source: 2, target: 1), new Link(index: 2, source: 1, target: 2)), + }; + + // Expected - The simplification should show only the initial-to-final transformations + var expectedSimplifiedChanges = new List<(Link Before, Link After)> + { + (new Link(index: 1, source: 1, target: 2), new Link(index: 1, source: 2, target: 1)), + (new Link(index: 2, source: 2, target: 1), new Link(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 Before, Link After)> + { + // Let's say we get these individual changes that don't form a proper chain + (new Link(index: 1, source: 1, target: 2), new Link(index: 0, source: 0, target: 0)), // delete + (new Link(index: 1, source: 1, target: 2), new Link(index: 1, source: 2, target: 1)), // direct update (this might be what's reported) + (new Link(index: 2, source: 2, target: 1), new Link(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 Before, Link After)> expectedSimplifiedChanges, List<(Link Before, Link After)> simplifiedChanges diff --git a/csharp/Foundation.Data.Doublets.Cli/ChangesSimplifier.cs b/csharp/Foundation.Data.Doublets.Cli/ChangesSimplifier.cs index 9b618d5..68cb8f9 100644 --- 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, Link)>(); } + // **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 Before, Link After)>(); var changedStates = new List<(Link Before, Link After)>(); @@ -124,6 +128,61 @@ public static class ChangesSimplifier .ThenBy(r => r.After.Target); } + /// + /// 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. + /// + /// The list of changes that may contain problematic duplicate before states + /// A list with problematic duplicates resolved + private static List<(Link Before, Link After)> RemoveDuplicateBeforeStates( + List<(Link Before, Link After)> changes) + { + // Group changes by their before state + var groupedChanges = changes.GroupBy(c => c.Before, LinkEqualityComparer.Instance); + + var result = new List<(Link Before, Link 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; + } + /// /// An equality comparer for Link that checks Index/Source/Target. /// diff --git a/csharp/Foundation.Data.Doublets.Cli/Program.cs b/csharp/Foundation.Data.Doublets.Cli/Program.cs index 1f9bfed..2021dc3 100644 --- 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 new file mode 100644 index 0000000..a19c16d --- /dev/null +++ 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 Before, Link 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(index: 1, source: 1, target: 2), new Link(index: 1, source: 2, target: 1)), + (new Link(index: 2, source: 2, target: 1), new Link(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})"); +} \ No newline at end of file diff --git a/examples/test1_output.txt b/examples/test1_output.txt new file mode 100644 index 0000000..e69de29 diff --git a/examples/test_issue_scenario.cs b/examples/test_issue_scenario.cs new file mode 100644 index 0000000..01c4c23 --- /dev/null +++ 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 Before, Link After)> +{ + // First transformation: (1: 1 2) -> () (deletion) + (new Link(index: 1, source: 1, target: 2), new Link(index: 0, source: 0, target: 0)), + + // Second transformation: () -> (1: 2 1) (creation) + (new Link(index: 0, source: 0, target: 0), new Link(index: 1, source: 2, target: 1)), + + // Third transformation: (2: 2 1) -> (2: 1 2) (direct update) + (new Link(index: 2, source: 2, target: 1), new Link(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."); \ No newline at end of file diff --git a/rust/src/changes_simplifier.rs b/rust/src/changes_simplifier.rs index 12815e5..8f2c966 100644 --- 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(); @@ -106,6 +110,54 @@ 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> = 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 +} + #[cfg(test)] mod tests { use super::*; @@ -150,4 +202,110 @@ mod tests { assert_eq!(result.len(), 2); } + + #[test] + fn test_simplify_issue26_update_operation() { + // 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 + let changes = vec![ + // Step 1: Link (1: 1 2) is first deleted (becomes null/empty) + (Link::new(1, 1, 2), Link::new(0, 0, 0)), + // Step 2: New link (1: 2 1) is created (swapped source and target) + (Link::new(0, 0, 0), Link::new(1, 2, 1)), + // Step 3: Link (2: 2 1) is directly updated to (2: 1 2) + (Link::new(2, 2, 1), Link::new(2, 1, 2)), + ]; + + let result = simplify_changes(changes); + + // Expected - The simplification should show only the initial-to-final transformations + assert_eq!(result.len(), 2); + assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1)))); + assert!(result.contains(&(Link::new(2, 2, 1), Link::new(2, 1, 2)))); + } + + #[test] + fn test_simplify_issue26_alternative_scenario() { + // This tests the scenario where the same before state has multiple transitions: + // one to null (deletion) and one to a non-null state (update) + let changes = vec![ + // Let's say we get these individual changes that don't form a proper chain + (Link::new(1, 1, 2), Link::new(0, 0, 0)), // delete + (Link::new(1, 1, 2), Link::new(1, 2, 1)), // direct update (this is what should be kept) + (Link::new(2, 2, 1), Link::new(2, 1, 2)), // direct update + ]; + + let result = simplify_changes(changes); + + // After the fix, we should prefer the non-null transition + // So we should get only 2 changes: the two direct updates + assert_eq!(result.len(), 2); + assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1)))); + assert!(result.contains(&(Link::new(2, 2, 1), Link::new(2, 1, 2)))); + } + + #[test] + fn test_simplify_multiple_branches_from_same_initial() { + // Original transitions (Before -> After): + // (0: 0 0) -> (1: 0 0) + // (1: 0 0) -> (1: 1 1) + // (0: 0 0) -> (2: 0 0) + // (2: 0 0) -> (2: 2 2) + let changes = vec![ + (Link::new(0, 0, 0), Link::new(1, 0, 0)), + (Link::new(1, 0, 0), Link::new(1, 1, 1)), + (Link::new(0, 0, 0), Link::new(2, 0, 0)), + (Link::new(2, 0, 0), Link::new(2, 2, 2)), + ]; + + let result = simplify_changes(changes); + + // Expected final transitions (After simplification): + // (0: 0 0) -> (1: 1 1) + // (0: 0 0) -> (2: 2 2) + assert_eq!(result.len(), 2); + assert!(result.contains(&(Link::new(0, 0, 0), Link::new(1, 1, 1)))); + assert!(result.contains(&(Link::new(0, 0, 0), Link::new(2, 2, 2)))); + } + + #[test] + fn test_simplify_specific_example_removes_intermediate_states() { + // (1: 2 1) ↦ (1: 0 0) + // (2: 1 2) ↦ (2: 0 0) + // (2: 0 0) ↦ (0: 0 0) + // (1: 0 0) ↦ (0: 0 0) + let changes = vec![ + (Link::new(1, 2, 1), Link::new(1, 0, 0)), + (Link::new(2, 1, 2), Link::new(2, 0, 0)), + (Link::new(2, 0, 0), Link::new(0, 0, 0)), + (Link::new(1, 0, 0), Link::new(0, 0, 0)), + ]; + + let result = simplify_changes(changes); + + // Expected simplified changes: + // (1: 2 1) ↦ (0: 0 0) + // (2: 1 2) ↦ (0: 0 0) + assert_eq!(result.len(), 2); + assert!(result.contains(&(Link::new(1, 2, 1), Link::new(0, 0, 0)))); + assert!(result.contains(&(Link::new(2, 1, 2), Link::new(0, 0, 0)))); + } + + #[test] + fn test_simplify_keeps_unchanged_states() { + // (1: 1 2) ↦ (1: 2 1) + // (2: 2 2) ↦ (2: 2 2) (unchanged) + let changes = vec![ + (Link::new(1, 1, 2), Link::new(1, 2, 1)), + (Link::new(2, 2, 2), Link::new(2, 2, 2)), + ]; + + let result = simplify_changes(changes); + + // Expected simplified changes still have (2: 2 2): + assert_eq!(result.len(), 2); + assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1)))); + assert!(result.contains(&(Link::new(2, 2, 2), Link::new(2, 2, 2)))); + } }