Skip to content

Commit 37e9657

Browse files
konardclaude
andcommitted
Apply Issue #26 fix to Rust implementation
- Added remove_duplicate_before_states() function to handle duplicate before states with conflicting transitions (null vs non-null) - When a before state has both a null transition (0: 0 0) and non-null transitions, prefer the non-null transitions as they represent the actual final transformations - Added comprehensive tests for Issue #26 scenarios: - test_simplify_issue26_update_operation - test_simplify_issue26_alternative_scenario - test_simplify_multiple_branches_from_same_initial - test_simplify_specific_example_removes_intermediate_states - test_simplify_keeps_unchanged_states - Both C# and Rust implementations now have symmetric logic 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d0299db commit 37e9657

File tree

1 file changed

+158
-0
lines changed

1 file changed

+158
-0
lines changed

rust/src/changes_simplifier.rs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ pub fn simplify_changes(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
1616
return vec![];
1717
}
1818

19+
// **FIX for Issue #26**: Remove duplicate before states by keeping the non-null transitions
20+
// This handles cases where the same link is reported with multiple different transformations
21+
let changes = remove_duplicate_before_states(changes);
22+
1923
// First, handle unchanged states directly
2024
let mut unchanged_states = Vec::new();
2125
let mut changed_states = Vec::new();
@@ -106,6 +110,54 @@ pub fn simplify_changes(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
106110
results
107111
}
108112

113+
/// Removes problematic duplicate before states that lead to simplification issues.
114+
/// This fixes Issue #26 where multiple transformations from the same before state
115+
/// to conflicting after states (including null states) would cause the simplifier to fail.
116+
///
117+
/// The key insight: If we have multiple transitions from the same before state,
118+
/// and one of them is to a "null" state (0: 0 0), we should prefer the non-null transition
119+
/// as it represents the actual final transformation.
120+
fn remove_duplicate_before_states(changes: Vec<(Link, Link)>) -> Vec<(Link, Link)> {
121+
// Group changes by their before state
122+
let mut grouped: HashMap<Link, Vec<(Link, Link)>> = HashMap::new();
123+
for change in changes {
124+
grouped.entry(change.0).or_default().push(change);
125+
}
126+
127+
let mut result = Vec::new();
128+
129+
for (_before, changes_for_this_before) in grouped {
130+
if changes_for_this_before.len() == 1 {
131+
// No duplicates, keep as is
132+
result.extend(changes_for_this_before);
133+
} else {
134+
// Multiple changes from the same before state
135+
// Check if any of them is to a null state (0: 0 0)
136+
let null_link = Link::new(0, 0, 0);
137+
let has_null_transition = changes_for_this_before
138+
.iter()
139+
.any(|(_, after)| *after == null_link);
140+
let non_null_transitions: Vec<_> = changes_for_this_before
141+
.iter()
142+
.filter(|(_, after)| *after != null_link)
143+
.cloned()
144+
.collect();
145+
146+
if has_null_transition && !non_null_transitions.is_empty() {
147+
// Issue #26 scenario: We have both null and non-null transitions
148+
// Prefer the non-null transitions as they represent the actual final states
149+
result.extend(non_null_transitions);
150+
} else {
151+
// No null transitions involved, this is a legitimate multiple-branch scenario
152+
// Keep all transitions
153+
result.extend(changes_for_this_before);
154+
}
155+
}
156+
}
157+
158+
result
159+
}
160+
109161
#[cfg(test)]
110162
mod tests {
111163
use super::*;
@@ -150,4 +202,110 @@ mod tests {
150202

151203
assert_eq!(result.len(), 2);
152204
}
205+
206+
#[test]
207+
fn test_simplify_issue26_update_operation() {
208+
// This represents the scenario described in GitHub issue #26
209+
// Where links (1: 1 2) and (2: 2 1) are being updated to swap source and target
210+
// The issue was that intermediate steps were being shown instead of the final transformation
211+
let changes = vec![
212+
// Step 1: Link (1: 1 2) is first deleted (becomes null/empty)
213+
(Link::new(1, 1, 2), Link::new(0, 0, 0)),
214+
// Step 2: New link (1: 2 1) is created (swapped source and target)
215+
(Link::new(0, 0, 0), Link::new(1, 2, 1)),
216+
// Step 3: Link (2: 2 1) is directly updated to (2: 1 2)
217+
(Link::new(2, 2, 1), Link::new(2, 1, 2)),
218+
];
219+
220+
let result = simplify_changes(changes);
221+
222+
// Expected - The simplification should show only the initial-to-final transformations
223+
assert_eq!(result.len(), 2);
224+
assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1))));
225+
assert!(result.contains(&(Link::new(2, 2, 1), Link::new(2, 1, 2))));
226+
}
227+
228+
#[test]
229+
fn test_simplify_issue26_alternative_scenario() {
230+
// This tests the scenario where the same before state has multiple transitions:
231+
// one to null (deletion) and one to a non-null state (update)
232+
let changes = vec![
233+
// Let's say we get these individual changes that don't form a proper chain
234+
(Link::new(1, 1, 2), Link::new(0, 0, 0)), // delete
235+
(Link::new(1, 1, 2), Link::new(1, 2, 1)), // direct update (this is what should be kept)
236+
(Link::new(2, 2, 1), Link::new(2, 1, 2)), // direct update
237+
];
238+
239+
let result = simplify_changes(changes);
240+
241+
// After the fix, we should prefer the non-null transition
242+
// So we should get only 2 changes: the two direct updates
243+
assert_eq!(result.len(), 2);
244+
assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1))));
245+
assert!(result.contains(&(Link::new(2, 2, 1), Link::new(2, 1, 2))));
246+
}
247+
248+
#[test]
249+
fn test_simplify_multiple_branches_from_same_initial() {
250+
// Original transitions (Before -> After):
251+
// (0: 0 0) -> (1: 0 0)
252+
// (1: 0 0) -> (1: 1 1)
253+
// (0: 0 0) -> (2: 0 0)
254+
// (2: 0 0) -> (2: 2 2)
255+
let changes = vec![
256+
(Link::new(0, 0, 0), Link::new(1, 0, 0)),
257+
(Link::new(1, 0, 0), Link::new(1, 1, 1)),
258+
(Link::new(0, 0, 0), Link::new(2, 0, 0)),
259+
(Link::new(2, 0, 0), Link::new(2, 2, 2)),
260+
];
261+
262+
let result = simplify_changes(changes);
263+
264+
// Expected final transitions (After simplification):
265+
// (0: 0 0) -> (1: 1 1)
266+
// (0: 0 0) -> (2: 2 2)
267+
assert_eq!(result.len(), 2);
268+
assert!(result.contains(&(Link::new(0, 0, 0), Link::new(1, 1, 1))));
269+
assert!(result.contains(&(Link::new(0, 0, 0), Link::new(2, 2, 2))));
270+
}
271+
272+
#[test]
273+
fn test_simplify_specific_example_removes_intermediate_states() {
274+
// (1: 2 1) ↦ (1: 0 0)
275+
// (2: 1 2) ↦ (2: 0 0)
276+
// (2: 0 0) ↦ (0: 0 0)
277+
// (1: 0 0) ↦ (0: 0 0)
278+
let changes = vec![
279+
(Link::new(1, 2, 1), Link::new(1, 0, 0)),
280+
(Link::new(2, 1, 2), Link::new(2, 0, 0)),
281+
(Link::new(2, 0, 0), Link::new(0, 0, 0)),
282+
(Link::new(1, 0, 0), Link::new(0, 0, 0)),
283+
];
284+
285+
let result = simplify_changes(changes);
286+
287+
// Expected simplified changes:
288+
// (1: 2 1) ↦ (0: 0 0)
289+
// (2: 1 2) ↦ (0: 0 0)
290+
assert_eq!(result.len(), 2);
291+
assert!(result.contains(&(Link::new(1, 2, 1), Link::new(0, 0, 0))));
292+
assert!(result.contains(&(Link::new(2, 1, 2), Link::new(0, 0, 0))));
293+
}
294+
295+
#[test]
296+
fn test_simplify_keeps_unchanged_states() {
297+
// (1: 1 2) ↦ (1: 2 1)
298+
// (2: 2 2) ↦ (2: 2 2) (unchanged)
299+
let changes = vec![
300+
(Link::new(1, 1, 2), Link::new(1, 2, 1)),
301+
(Link::new(2, 2, 2), Link::new(2, 2, 2)),
302+
];
303+
304+
let result = simplify_changes(changes);
305+
306+
// Expected simplified changes still have (2: 2 2):
307+
assert_eq!(result.len(), 2);
308+
assert!(result.contains(&(Link::new(1, 1, 2), Link::new(1, 2, 1))));
309+
assert!(result.contains(&(Link::new(2, 2, 2), Link::new(2, 2, 2))));
310+
}
153311
}

0 commit comments

Comments
 (0)