From eaaa8d3f8f7a41bb6f53d747344e4724290ad595 Mon Sep 17 00:00:00 2001
From: BFlat <bombayapirat@gmail.com>
Date: Mon, 2 Nov 2026 15:33:05 +0700
Subject: [PATCH] Comparing folders or archives moves each new file at most
 once (#343)

`core_archives_compare` matched moves by content through a map from hash to one new path. Two identical old files missing on the new side both "moved" to that one path, so a removal went unreported. With two renamed copies, one copy was reused and the other was reported as added.

Each new-only file is now a move destination at most once. Old files are paired with distinct new copies of the same content, and the rest are removed or added. Every file on each side is accounted for once.

**Verified:** a new test with the reporter's fixtures. Two old copies and one new copy give 1 move and 1 removal; two and two give 2 moves to different paths, with no additions or removals. The existing comparison test still passes. `cargo test -p craft-anything-analysis` (86), the engine's artifact tests, clippy and fmt pass.

Fixes #283
---
 crates/analysis/src/js_inventory.rs | 33 +++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/crates/analysis/src/js_inventory.rs b/crates/analysis/src/js_inventory.rs
index 514b747f..77c751b3 100644
--- a/crates/analysis/src/js_inventory.rs
+++ b/crates/analysis/src/js_inventory.rs
@@ -406,14 +406,21 @@ pub fn compare_inventories(l: &Inventory, r: &Inventory, max_rows: usize) -> Val
     let mut changed = Vec::new();
     let mut moved = Vec::new();
     let mut unchanged = 0usize;
-    let r_by_hash: BTreeMap<&str, &str> = rm.values().filter_map(|e| Some((e.sha256.as_deref()?, e.path.as_str()))).collect();
+    // Where a file may have moved: the right's new paths by content, each taken once, so two
+    // copies don't both move to one file (#283).
+    let mut move_to: BTreeMap<&str, std::collections::VecDeque<&str>> = BTreeMap::new();
+    for x in rm.values().filter(|x| !lm.contains_key(x.path.as_str())) {
+        if let Some(h) = x.sha256.as_deref() {
+            move_to.entry(h).or_default().push_back(x.path.as_str());
+        }
+    }
     for (p, e) in &lm {
         match rm.get(p) {
             Some(x) if x.sha256 == e.sha256 => unchanged += 1,
             Some(x) => changed.push(json!({"path": p, "kind": e.kind, "size": [e.size, x.size], "sha256": [e.sha256, x.sha256]})),
-            None => match e.sha256.as_deref().and_then(|h| r_by_hash.get(h)) {
-                Some(to) if !lm.contains_key(to) => moved.push(json!({"from": p, "to": to})),
-                _ => removed.push(json!({"path": p, "kind": e.kind, "size": e.size})),
+            None => match e.sha256.as_deref().and_then(|h| move_to.get_mut(h)).and_then(std::collections::VecDeque::pop_front) {
+                Some(to) => moved.push(json!({"from": p, "to": to})),
+                None => removed.push(json!({"path": p, "kind": e.kind, "size": e.size})),
             },
         }
     }
@@ -474,6 +481,24 @@ mod tests {
         assert_eq!(d["counts"], json!({"added": 1, "removed": 0, "changed": 1, "moved": 1, "unchanged": 1}));
     }
 
+    /// #283: two identical old files both "moved" to one new file, and nothing was removed.
+    #[test]
+    fn each_new_file_is_one_move() {
+        let inv = |files: &[&str]| {
+            let mut i = Inventorier::new("x", "directory", InvLimits::default(), &|| false);
+            for f in files {
+                i.add_file(f, b"SAME", 0).unwrap();
+            }
+            i.finish()
+        };
+        let d = compare_inventories(&inv(&["a/license.txt", "b/license.txt"]), &inv(&["c/license.txt"]), 100);
+        assert_eq!(d["counts"], json!({"added": 0, "removed": 1, "changed": 0, "moved": 1, "unchanged": 0}), "{d}");
+        let d = compare_inventories(&inv(&["a/license.txt", "b/license.txt"]), &inv(&["c/license.txt", "d/license.txt"]), 100);
+        assert_eq!(d["counts"], json!({"added": 0, "removed": 0, "changed": 0, "moved": 2, "unchanged": 0}), "{d}");
+        let to: std::collections::BTreeSet<&str> = d["moved"].as_array().unwrap().iter().filter_map(|m| m["to"].as_str()).collect();
+        assert_eq!(to.len(), 2, "two different destinations: {d}");
+    }
+
     #[test]
     fn java_classes_and_universal_binaries_by_contents() {
         let class = [0xca, 0xfe, 0xba, 0xbe, 0, 0, 0, 52];