fix: address four correctness bugs in duplicate detection and tagging
- Duplicate scanner now runs a full-file hash pass (Phase 3) for large files (> 64 KB) after the sample-hash shortlist, preventing false positives from reaching the destructive deletion workflow - delete_images_from_disk now attempts filesystem removal before touching the DB; only successfully deleted files have their rows removed, keeping locked/permission-denied files visible in the library - AI tag upsert guards user-sourced rows with WHERE source != 'user' so a tagger re-run cannot silently overwrite manually added tags - clear_tagging_jobs excludes 'cancelled' rows from the immediate DELETE so the worker can observe cancellation via is_tagging_job_cancelled before the rows are cleaned up at next startup
This commit is contained in:
+83
-26
@@ -857,7 +857,7 @@ pub async fn find_duplicates(
|
|||||||
let total = records.len();
|
let total = records.len();
|
||||||
let _ = app.emit("duplicate_scan_progress", (0usize, total));
|
let _ = app.emit("duplicate_scan_progress", (0usize, total));
|
||||||
|
|
||||||
// Two-phase detection. No full-file read at any point.
|
// Three-phase detection.
|
||||||
//
|
//
|
||||||
// Phase 1 — stat:
|
// Phase 1 — stat:
|
||||||
// Read only file metadata (size). Files with a unique size cannot be
|
// Read only file metadata (size). Files with a unique size cannot be
|
||||||
@@ -865,15 +865,14 @@ pub async fn find_duplicates(
|
|||||||
//
|
//
|
||||||
// Phase 2 — sample hash:
|
// Phase 2 — sample hash:
|
||||||
// For each size-matched candidate, read four evenly-spaced 16 KB windows
|
// For each size-matched candidate, read four evenly-spaced 16 KB windows
|
||||||
// (head, 33%, 66%, tail) and hash them together. Total I/O per file is
|
// and hash them. For small files (≤ 64 KB) the windows cover the whole
|
||||||
// capped at 64 KB regardless of how large the file is.
|
// file so the hash is exact. For large files this is a fast shortlist.
|
||||||
//
|
//
|
||||||
// For small files (≤ 64 KB) the windows cover the whole file, so the
|
// Phase 3 — full hash (large files only):
|
||||||
// hash is exact. For large files, matching all four windows at different
|
// For sample-hash groups that contain files > 64 KB, compute a full-file
|
||||||
// offsets is effectively impossible for natural photo/video content unless
|
// hash to confirm the match before presenting them as deletable duplicates.
|
||||||
// the files are genuinely identical — eliminating the need for a full read.
|
|
||||||
let app_hash = app.clone();
|
let app_hash = app.clone();
|
||||||
let pairs: Vec<(u64, u64, i64)> = tokio::task::spawn_blocking(move || {
|
let pairs: Vec<(u64, u64, i64, String)> = tokio::task::spawn_blocking(move || {
|
||||||
use memmap2::Mmap;
|
use memmap2::Mmap;
|
||||||
use rayon::prelude::*;
|
use rayon::prelude::*;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
@@ -939,23 +938,82 @@ pub async fn find_duplicates(
|
|||||||
if done % 100 == 0 || done == c_total {
|
if done % 100 == 0 || done == c_total {
|
||||||
let _ = app_hash.emit("duplicate_scan_progress", (done, c_total));
|
let _ = app_hash.emit("duplicate_scan_progress", (done, c_total));
|
||||||
}
|
}
|
||||||
Some((hash, *size, *id))
|
Some((hash, *size, *id, path.clone()))
|
||||||
})
|
})
|
||||||
.collect()
|
.collect()
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
.map_err(|e| e.to_string())?;
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
let mut size_hash_map: std::collections::HashMap<(u64, u64), Vec<i64>> = std::collections::HashMap::new();
|
// Group by (sample_hash, file_size).
|
||||||
for (hash, file_size, id) in pairs {
|
let mut size_hash_map: std::collections::HashMap<(u64, u64), Vec<(i64, String)>> =
|
||||||
size_hash_map.entry((hash, file_size)).or_default().push(id);
|
std::collections::HashMap::new();
|
||||||
|
for (hash, file_size, id, path) in pairs {
|
||||||
|
size_hash_map.entry((hash, file_size)).or_default().push((id, path));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Phase 3: for large-file groups (> 64 KB) the sample hash is not exact;
|
||||||
|
// re-hash the full file contents to avoid false positives before deletion.
|
||||||
|
const COVERED: u64 = (16 * 1024 * 4) as u64;
|
||||||
|
let confirmed: std::collections::HashMap<(u64, u64), Vec<i64>> =
|
||||||
|
tokio::task::spawn_blocking(move || {
|
||||||
|
use memmap2::Mmap;
|
||||||
|
use rayon::prelude::*;
|
||||||
|
use xxhash_rust::xxh3::xxh3_64;
|
||||||
|
|
||||||
|
size_hash_map
|
||||||
|
.into_iter()
|
||||||
|
.filter(|(_, entries)| entries.len() > 1)
|
||||||
|
.filter_map(|((sample_hash, file_size), entries)| {
|
||||||
|
if file_size <= COVERED {
|
||||||
|
// Sample was already a full hash — no re-read needed.
|
||||||
|
let ids = entries.into_iter().map(|(id, _)| id).collect();
|
||||||
|
return Some(((sample_hash, file_size), ids));
|
||||||
|
}
|
||||||
|
// Full-file hash pass to confirm.
|
||||||
|
let full_hashes: Vec<(i64, Option<u64>)> = entries
|
||||||
|
.par_iter()
|
||||||
|
.map(|(id, path)| {
|
||||||
|
let hash = std::fs::File::open(path)
|
||||||
|
.ok()
|
||||||
|
.and_then(|f| unsafe { Mmap::map(&f).ok() })
|
||||||
|
.map(|mmap| xxh3_64(&mmap));
|
||||||
|
(*id, hash)
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
// Group by full hash; keep only groups where every file hashed
|
||||||
|
// successfully and at least two share the same hash.
|
||||||
|
let mut by_full: std::collections::HashMap<u64, Vec<i64>> =
|
||||||
|
std::collections::HashMap::new();
|
||||||
|
for (id, maybe_hash) in full_hashes {
|
||||||
|
if let Some(h) = maybe_hash {
|
||||||
|
by_full.entry(h).or_default().push(id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Return the largest confirmed group (same sample_hash/size bucket).
|
||||||
|
// In practice a single full hash will dominate; emit each sub-group.
|
||||||
|
// We flatten into one entry using the sample_hash key — callers only
|
||||||
|
// need confirmed IDs, not the exact full hash.
|
||||||
|
let confirmed_ids: Vec<i64> = by_full
|
||||||
|
.into_values()
|
||||||
|
.filter(|g| g.len() > 1)
|
||||||
|
.flatten()
|
||||||
|
.collect();
|
||||||
|
if confirmed_ids.is_empty() {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(((sample_hash, file_size), confirmed_ids))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
// Resolve image records for each duplicate group
|
// Resolve image records for each duplicate group
|
||||||
let conn = db.get().map_err(|e| e.to_string())?;
|
let conn = db.get().map_err(|e| e.to_string())?;
|
||||||
let mut groups: Vec<DuplicateGroup> = size_hash_map
|
let mut groups: Vec<DuplicateGroup> = confirmed
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter(|(_, ids)| ids.len() > 1)
|
|
||||||
.filter_map(|((hash, file_size), ids)| {
|
.filter_map(|((hash, file_size), ids)| {
|
||||||
let images = db::get_images_by_ids(&conn, &ids).ok()?;
|
let images = db::get_images_by_ids(&conn, &ids).ok()?;
|
||||||
Some(DuplicateGroup {
|
Some(DuplicateGroup {
|
||||||
@@ -1010,23 +1068,22 @@ pub async fn delete_images_from_disk(
|
|||||||
return Ok(0);
|
return Ok(0);
|
||||||
}
|
}
|
||||||
let conn = db.get().map_err(|e| e.to_string())?;
|
let conn = db.get().map_err(|e| e.to_string())?;
|
||||||
// Collect paths before deleting DB rows
|
|
||||||
let records = db::get_all_image_paths(&conn, None).map_err(|e| e.to_string())?;
|
let records = db::get_all_image_paths(&conn, None).map_err(|e| e.to_string())?;
|
||||||
let id_set: std::collections::HashSet<i64> = params.image_ids.iter().copied().collect();
|
let id_set: std::collections::HashSet<i64> = params.image_ids.iter().copied().collect();
|
||||||
let paths: Vec<String> = records
|
|
||||||
.into_iter()
|
|
||||||
.filter(|r| id_set.contains(&r.id))
|
|
||||||
.map(|r| r.path)
|
|
||||||
.collect();
|
|
||||||
|
|
||||||
db::delete_images_by_ids(&conn, ¶ms.image_ids).map_err(|e| e.to_string())?;
|
// Attempt filesystem removal first; only delete DB rows for files that
|
||||||
|
// were successfully removed. This prevents entries from disappearing from
|
||||||
let mut deleted = 0usize;
|
// the library when a file is locked or permission-denied.
|
||||||
for path in &paths {
|
let mut succeeded_ids: Vec<i64> = Vec::new();
|
||||||
if std::fs::remove_file(path).is_ok() {
|
for r in records.into_iter().filter(|r| id_set.contains(&r.id)) {
|
||||||
deleted += 1;
|
if std::fs::remove_file(&r.path).is_ok() {
|
||||||
|
succeeded_ids.push(r.id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
let deleted = succeeded_ids.len();
|
||||||
|
if !succeeded_ids.is_empty() {
|
||||||
|
db::delete_images_by_ids(&conn, &succeeded_ids).map_err(|e| e.to_string())?;
|
||||||
|
}
|
||||||
Ok(deleted)
|
Ok(deleted)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+4
-3
@@ -1697,7 +1697,8 @@ pub fn update_ai_tags(
|
|||||||
ON CONFLICT(image_id, tag) DO UPDATE SET
|
ON CONFLICT(image_id, tag) DO UPDATE SET
|
||||||
source = 'ai',
|
source = 'ai',
|
||||||
ai_model = excluded.ai_model,
|
ai_model = excluded.ai_model,
|
||||||
confidence = excluded.confidence",
|
confidence = excluded.confidence
|
||||||
|
WHERE source != 'user'",
|
||||||
)?;
|
)?;
|
||||||
for (tag, confidence) in tags {
|
for (tag, confidence) in tags {
|
||||||
stmt.execute(params![image_id, tag, model, confidence])?;
|
stmt.execute(params![image_id, tag, model, confidence])?;
|
||||||
@@ -1748,7 +1749,7 @@ pub fn clear_tagging_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<u
|
|||||||
)?;
|
)?;
|
||||||
let n = conn.execute(
|
let n = conn.execute(
|
||||||
"DELETE FROM tagging_jobs
|
"DELETE FROM tagging_jobs
|
||||||
WHERE status != 'processing'
|
WHERE status NOT IN ('processing', 'cancelled')
|
||||||
AND image_id IN (SELECT id FROM images WHERE folder_id = ?1)",
|
AND image_id IN (SELECT id FROM images WHERE folder_id = ?1)",
|
||||||
[fid],
|
[fid],
|
||||||
)?;
|
)?;
|
||||||
@@ -1767,7 +1768,7 @@ pub fn clear_tagging_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<u
|
|||||||
WHERE status = 'processing'",
|
WHERE status = 'processing'",
|
||||||
[],
|
[],
|
||||||
)?;
|
)?;
|
||||||
let n = conn.execute("DELETE FROM tagging_jobs WHERE status != 'processing'", [])?;
|
let n = conn.execute("DELETE FROM tagging_jobs WHERE status NOT IN ('processing', 'cancelled')", [])?;
|
||||||
conn.execute(
|
conn.execute(
|
||||||
"UPDATE images SET ai_tagger_error = NULL WHERE ai_tagged_at IS NULL",
|
"UPDATE images SET ai_tagger_error = NULL WHERE ai_tagged_at IS NULL",
|
||||||
[],
|
[],
|
||||||
|
|||||||
Reference in New Issue
Block a user