From 9ec024ddceda65fb5f1df85deee484ac450ba3f2 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sat, 22 Aug 2026 21:41:57 +0000 Subject: [PATCH 1/3] diff_3way: stash side-tagged removed/added pools --- src/engine/mod.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 1c61e2a..3c256dd 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -223,6 +223,14 @@ impl Engine { let mut pending_removed: Vec<(String, FnDef)> = Vec::new(); let empty: Vec = Vec::new(); + // Every disappearance/appearance relative to base, tagged + // by side and stashed before any dispatch decision runs. + // Stashing here changes no existing emissions. + let mut removed_ours: Vec<(String, usize, FnDef)> = Vec::new(); + let mut added_ours: Vec<(String, FnDef)> = Vec::new(); + let mut removed_theirs: Vec<(String, usize, FnDef)> = Vec::new(); + let mut added_theirs: Vec<(String, FnDef)> = Vec::new(); + for name in all_fn_names { let b_list = b_fns.get(name).unwrap_or(&empty); let o_list = o_fns.get(name).unwrap_or(&empty); @@ -231,6 +239,19 @@ impl Engine { let (o_changed, o_gone, o_fresh) = align_defs(b_list, o_list); let (t_changed, t_gone, t_fresh) = align_defs(b_list, t_list); + for bi in &o_gone { + removed_ours.push((name.clone(), *bi, b_list[*bi].clone())); + } + for hi in &o_fresh { + added_ours.push((name.clone(), o_list[*hi].clone())); + } + for bi in &t_gone { + removed_theirs.push((name.clone(), *bi, b_list[*bi].clone())); + } + for ti in &t_fresh { + added_theirs.push((name.clone(), t_list[*ti].clone())); + } + let ours_touched = !(o_changed.is_empty() && o_gone.is_empty() && o_fresh.is_empty()); let theirs_touched = @@ -341,6 +362,15 @@ impl Engine { disputes.push(meaning(&mut n, path, row, detail, Severity::High)); } + // Detection runs now; its results start driving + // emission/suppression with the divergence work itself. + let _pending_divergent_renames = find_divergent_renames( + &removed_ours, + &added_ours, + &removed_theirs, + &added_theirs, + ); + // Pair incoming removals with incoming additions of // identical blanked-name source text: a rename, not two // changes. @@ -508,6 +538,90 @@ fn strip_common<'a>(a: &'a [FnDef], b: &[FnDef]) -> Vec<&'a FnDef> { leftover } +/// Rename compatibility between a removed def's signature and an added +/// def's signature: `Some` on exact equality today. A future similarity +/// metric widens this to graded scores without touching the pairing logic. +fn rename_score(candidate: &str, original: &str) -> Option<()> { + (candidate == original).then_some(()) +} + +/// One side's rename evidence: a base def identified by `(name, index +/// within its base group)` that this side deleted, and the new name/row it +/// reappeared under. +struct SidePair { + old_name: String, + old_idx: usize, + new_name: String, + new_row: usize, +} + +/// A base definition both branches renamed, to different names. +#[derive(Debug, Clone, PartialEq, Eq)] +struct DivergentRename { + base_name: String, + ours_new_name: String, + ours_row: usize, + theirs_new_name: String, + theirs_row: usize, +} + +/// Greedily pair each removed def with the first unconsumed added def whose +/// signature matches exactly under a different name. Entries are consumed +/// once on both sides, so duplicates pair one-to-one. +fn pair_side_renames( + removed: &[(String, usize, FnDef)], + added: &[(String, FnDef)], +) -> Vec { + let mut used = vec![false; added.len()]; + let mut pairs = Vec::new(); + for (old_name, old_idx, old_def) in removed { + let found = added.iter().enumerate().find(|(i, (new_name, new_def))| { + !used[*i] + && new_name != old_name + && rename_score(&new_def.signature, &old_def.signature).is_some() + }); + if let Some((i, (new_name, new_def))) = found { + used[i] = true; + pairs.push(SidePair { + old_name: old_name.clone(), + old_idx: *old_idx, + new_name: new_name.clone(), + new_row: new_def.row, + }); + } + } + pairs +} + +/// Detect definitions both branches renamed to different names: pair +/// removals to additions within each side, then join across sides on +/// `(base name, base index)`. Fires only when both sides paired and the new +/// names differ; convergent renames join on equal names and stay silent. +fn find_divergent_renames( + removed_ours: &[(String, usize, FnDef)], + added_ours: &[(String, FnDef)], + removed_theirs: &[(String, usize, FnDef)], + added_theirs: &[(String, FnDef)], +) -> Vec { + let ours_pairs = pair_side_renames(removed_ours, added_ours); + let theirs_pairs = pair_side_renames(removed_theirs, added_theirs); + let mut divergent = Vec::new(); + for tp in &theirs_pairs { + if let Some(op) = ours_pairs.iter().find(|op| { + op.old_name == tp.old_name && op.old_idx == tp.old_idx && op.new_name != tp.new_name + }) { + divergent.push(DivergentRename { + base_name: tp.old_name.clone(), + ours_new_name: op.new_name.clone(), + ours_row: op.new_row, + theirs_new_name: tp.new_name.clone(), + theirs_row: tp.new_row, + }); + } + } + divergent +} + fn parse_source(parser: &mut Parser, language: &Language, source: &str) -> Option { parser.set_language(language).ok()?; parser.parse(source, None) From 7d024b0049c45bd7cb3bc4e1f1e42acf5e6325e2 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sat, 22 Aug 2026 21:43:50 +0000 Subject: [PATCH 2/3] Rename/rename divergence: emit High instead of swallowing --- src/engine/mod.rs | 140 ++++++++++++++++++++++++++++++++++++++++++- tests/engine_test.rs | 137 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 274 insertions(+), 3 deletions(-) diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 3c256dd..aff56cd 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -362,14 +362,50 @@ impl Engine { disputes.push(meaning(&mut n, path, row, detail, Severity::High)); } - // Detection runs now; its results start driving - // emission/suppression with the divergence work itself. - let _pending_divergent_renames = find_divergent_renames( + // A base def both branches renamed to different names is + // the swallowed rename/rename dispute: one High per such + // def, located at theirs' new copy (fallback ours, then 0). + let divergent_renames = find_divergent_renames( &removed_ours, &added_ours, &removed_theirs, &added_theirs, ); + let mut claimed_theirs: Vec<(&str, usize)> = Vec::new(); + for d in &divergent_renames { + let row = if d.theirs_row > 0 { + d.theirs_row + } else if d.ours_row > 0 { + d.ours_row + } else { + 0 + }; + disputes.push(meaning( + &mut n, + path, + row, + format!( + "3-way conflict: both branches renamed function `{}` differently \ + (`{}` -> `{}` in target, `{}` -> `{}` in incoming)", + d.base_name, + d.base_name, + d.ours_new_name, + d.base_name, + d.theirs_new_name + ), + Severity::High, + )); + claimed_theirs.push((&d.theirs_new_name, d.theirs_row)); + } + // Defs already reported as the incoming half of a + // divergent rename must not resurface as Low additions. + if !claimed_theirs.is_empty() { + pending_added.retain(|(name, def)| { + !claimed_theirs + .iter() + .any(|(cn, row)| *cn == name.as_str() && *row == def.row) + }); + } // Pair incoming removals with incoming additions of // identical blanked-name source text: a rename, not two @@ -1229,4 +1265,102 @@ mod tests { assert_eq!(disputes.len(), 2); assert!(disputes.iter().all(|d| d.severity == Severity::High)); } + + fn rm(name: &str, idx: usize, sig: &str) -> (String, usize, FnDef) { + ( + name.to_string(), + idx, + FnDef { + src: String::new(), + row: 1, + signature: sig.to_string(), + }, + ) + } + + fn ad(name: &str, row: usize, sig: &str) -> (String, FnDef) { + ( + name.to_string(), + FnDef { + src: String::new(), + row, + signature: sig.to_string(), + }, + ) + } + + #[test] + fn test_rename_score_is_exact_only() { + assert!(rename_score("fn () {}", "fn () {}").is_some()); + assert!(rename_score("fn () {}", "fn (x) {}").is_none()); + } + + #[test] + fn test_pair_side_consumes_each_added_once() { + let removed = vec![rm("f", 0, "sigA"), rm("f", 1, "sigA")]; + let added = vec![ad("x", 3, "sigA")]; + let pairs = pair_side_renames(&removed, &added); + assert_eq!(pairs.len(), 1, "one added def cannot serve two removals"); + assert_eq!(pairs[0].old_idx, 0); + assert_eq!(pairs[0].new_name, "x"); + } + + #[test] + fn test_pair_side_matches_duplicates_one_to_one() { + let removed = vec![rm("f", 0, "sigA"), rm("f", 1, "sigB")]; + let added = vec![ad("y", 5, "sigB"), ad("x", 3, "sigA")]; + let pairs = pair_side_renames(&removed, &added); + assert_eq!(pairs.len(), 2); + assert!(pairs.iter().any(|p| p.old_idx == 0 && p.new_name == "x")); + assert!(pairs.iter().any(|p| p.old_idx == 1 && p.new_name == "y")); + } + + #[test] + fn test_pair_side_rejects_equal_names() { + // Same name on both sides is no rename; it is handled by the + // regular changed/gone alignment. + let removed = vec![rm("f", 0, "sigA")]; + let added = vec![ad("f", 3, "sigA")]; + assert!(pair_side_renames(&removed, &added).is_empty()); + } + + #[test] + fn test_find_divergent_requires_both_sides_and_different_names() { + let removed_ours = vec![rm("f", 0, "sigA")]; + let added_ours = vec![ad("g", 2, "sigA")]; + let removed_theirs = vec![rm("f", 0, "sigA")]; + + // Convergent rename: both sides landed on `g`, stays silent. + let added_theirs_convergent = vec![ad("g", 4, "sigA")]; + assert!(find_divergent_renames( + &removed_ours, + &added_ours, + &removed_theirs, + &added_theirs_convergent, + ) + .is_empty()); + + // Theirs deleted without renaming: no join. + assert!( + find_divergent_renames(&removed_ours, &added_ours, &removed_theirs, &[],).is_empty() + ); + + // Divergent: ours f -> g, theirs f -> k. + let added_theirs_divergent = vec![ad("k", 7, "sigA")]; + assert_eq!( + find_divergent_renames( + &removed_ours, + &added_ours, + &removed_theirs, + &added_theirs_divergent, + ), + vec![DivergentRename { + base_name: "f".into(), + ours_new_name: "g".into(), + ours_row: 2, + theirs_new_name: "k".into(), + theirs_row: 7, + }] + ); + } } diff --git a/tests/engine_test.rs b/tests/engine_test.rs index c969d83..2bc36a5 100644 --- a/tests/engine_test.rs +++ b/tests/engine_test.rs @@ -651,3 +651,140 @@ func (b B) Name() string { assert_eq!(disputes.len(), 1, "got {:?}", disputes); assert_eq!(disputes[0].detail, "both sides changed `Name`"); } + +#[test] +fn test_engine_3way_rename_rename_divergence_is_high_with_pinned_detail() { + let engine = Engine::new().expect("Failed to initialize engine"); + + let snap = |s: &str| { + let mut x = Snapshot::default(); + x.files + .insert("src/lib.rs".to_string(), s.as_bytes().to_vec()); + x + }; + + let disputes = engine + .diff_3way( + &snap("pub fn f() -> i32 { 1 }\n"), + &snap("pub fn g() -> i32 { 1 }\n"), + &snap("pub fn k() -> i32 { 1 }\n"), + ) + .expect("Diff failed"); + + assert_eq!(disputes.len(), 1, "got {:?}", disputes); + assert_eq!(disputes[0].severity, Severity::High); + assert_eq!( + disputes[0].detail, + "3-way conflict: both branches renamed function `f` differently (`f` -> `g` in target, `f` -> `k` in incoming)" + ); + assert_eq!( + disputes[0].location, "src/lib.rs:1", + "located at theirs' new copy" + ); +} + +#[test] +fn test_engine_3way_delete_delete_with_coincidental_adds_is_unchanged() { + let engine = Engine::new().expect("Failed to initialize engine"); + + let snap = |s: &str| { + let mut x = Snapshot::default(); + x.files + .insert("src/lib.rs".to_string(), s.as_bytes().to_vec()); + x + }; + + // Both sides delete `f`; each happens to add an unrelated helper with a + // different body. No rename pairing anywhere, so nothing may change: + // no High fabricated, theirs' addition still reports as Low. + let disputes = engine + .diff_3way( + &snap("pub fn f() -> i32 { 1 }\n"), + &snap("pub fn p() -> i32 { 11 }\n"), + &snap("pub fn q() -> i32 { 22 }\n"), + ) + .expect("Diff failed"); + + assert_eq!(disputes.len(), 1, "got {:?}", disputes); + assert_eq!(disputes[0].severity, Severity::Low); + assert_eq!(disputes[0].detail, "incoming branch added function `q`"); +} + +#[test] +fn test_engine_3way_convergent_rename_is_clean() { + let engine = Engine::new().expect("Failed to initialize engine"); + + let snap = |s: &str| { + let mut x = Snapshot::default(); + x.files + .insert("src/lib.rs".to_string(), s.as_bytes().to_vec()); + x + }; + + let disputes = engine + .diff_3way( + &snap("pub fn f() -> i32 { 1 }\n"), + &snap("pub fn g() -> i32 { 1 }\n"), + &snap("pub fn g() -> i32 { 1 }\n"), + ) + .expect("Diff failed"); + + assert!(disputes.is_empty(), "got {:?}", disputes); +} + +#[test] +fn test_engine_3way_two_divergent_renames_emit_two_highs() { + let engine = Engine::new().expect("Failed to initialize engine"); + + let snap = |s: &str| { + let mut x = Snapshot::default(); + x.files + .insert("src/lib.rs".to_string(), s.as_bytes().to_vec()); + x + }; + + let disputes = engine + .diff_3way( + &snap("pub fn f() -> i32 { 1 }\npub fn h() -> i32 { 2 }\n"), + &snap("pub fn g() -> i32 { 1 }\npub fn h2() -> i32 { 2 }\n"), + &snap("pub fn k() -> i32 { 1 }\npub fn h3() -> i32 { 2 }\n"), + ) + .expect("Diff failed"); + + assert_eq!(disputes.len(), 2, "got {:?}", disputes); + assert!(disputes.iter().all(|d| d.severity == Severity::High)); + assert!(disputes.iter().any(|d| d.detail + == "3-way conflict: both branches renamed function `f` differently (`f` -> `g` in target, `f` -> `k` in incoming)")); + assert!(disputes.iter().any(|d| d.detail + == "3-way conflict: both branches renamed function `h` differently (`h` -> `h2` in target, `h` -> `h3` in incoming)")); +} + +#[test] +fn test_engine_3way_rename_vs_body_edit_reports_modify_delete_gap() { + let engine = Engine::new().expect("Failed to initialize engine"); + + let snap = |s: &str| { + let mut x = Snapshot::default(); + x.files + .insert("src/lib.rs".to_string(), s.as_bytes().to_vec()); + x + }; + + // Known gap: ours renames f -> g while theirs edits f in place. The + // exact-signature detector cannot pair the edited def, so this reports + // modify/delete and stays silent about g until similarity scoring lands. + let disputes = engine + .diff_3way( + &snap("pub fn f() -> i32 { 1 }\n"), + &snap("pub fn g() -> i32 { 1 }\n"), + &snap("pub fn f() -> i32 { 42 }\n"), + ) + .expect("Diff failed"); + + assert_eq!(disputes.len(), 1, "got {:?}", disputes); + assert_eq!(disputes[0].severity, Severity::High); + assert_eq!( + disputes[0].detail, + "3-way conflict: function `f` modified in incoming branch but deleted in target" + ); +} From 08a6e923a8c265f01e19e413206981afbdb88a81 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sat, 22 Aug 2026 21:43:58 +0000 Subject: [PATCH 3/3] TODO: PR-O1 marked done in night-run status --- TODO.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index 9c58c48..a5ac394 100644 --- a/TODO.md +++ b/TODO.md @@ -297,7 +297,7 @@ court; the declared direction is that it becomes the source control itself." ## Night-run status -- [ ] PR-O1 fix/rename-rename-dispute +- [x] PR-O1 fix/rename-rename-dispute - [ ] PR-O2 docs/positioning - [ ] PR-O3 feat/store-court - [ ] PR-O4 feat/oot-update