fix: four more correctness bugs from PR review

- Duplicate scanner now emits one DuplicateGroup per distinct full hash
  within a sample-hash bucket, preventing disjoint sets (A,A vs B,B)
  from being merged and presented as a single deletable group
- Tagger model download now installs the shared ONNX runtime DLLs via
  ensure_onnx_runtime so the tagger is ready on a clean install without
  requiring the caption model to have been downloaded first
- Caption queue clear now follows the tagging worker pattern: marks
  in-flight rows as cancelled and deletes only non-processing rows;
  the caption worker skips writing results for cancelled jobs; startup
  cleanup removes any leftover cancelled caption rows
- Region search pagination now stores the crop rect in state and uses
  find_similar_by_region for subsequent pages instead of falling through
  to loadImages which appended unrelated folder results
This commit is contained in:
2026-06-06 22:00:20 +01:00
parent f82002129a
commit de0c2ab12d
5 changed files with 91 additions and 31 deletions
+16 -24
View File
@@ -953,9 +953,11 @@ pub async fn find_duplicates(
} }
// Phase 3: for large-file groups (> 64 KB) the sample hash is not exact; // 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. // re-hash the full file contents to confirm matches. Each distinct full
// hash becomes its own DuplicateGroup so disjoint sets (A,A vs B,B that
// happened to share size and sample hash) are never merged.
const COVERED: u64 = (16 * 1024 * 4) as u64; const COVERED: u64 = (16 * 1024 * 4) as u64;
let confirmed: std::collections::HashMap<(u64, u64), Vec<i64>> = let confirmed: Vec<(u64, u64, Vec<i64>)> =
tokio::task::spawn_blocking(move || { tokio::task::spawn_blocking(move || {
use memmap2::Mmap; use memmap2::Mmap;
use rayon::prelude::*; use rayon::prelude::*;
@@ -964,13 +966,13 @@ pub async fn find_duplicates(
size_hash_map size_hash_map
.into_iter() .into_iter()
.filter(|(_, entries)| entries.len() > 1) .filter(|(_, entries)| entries.len() > 1)
.filter_map(|((sample_hash, file_size), entries)| { .flat_map(|((sample_hash, file_size), entries)| {
if file_size <= COVERED { if file_size <= COVERED {
// Sample was already a full hash — no re-read needed. // Sample was already a full hash — one group, no re-read needed.
let ids = entries.into_iter().map(|(id, _)| id).collect(); return vec![(sample_hash, file_size, entries.into_iter().map(|(id, _)| id).collect())];
return Some(((sample_hash, file_size), ids));
} }
// Full-file hash pass to confirm. // Full-file hash pass. Group by full hash so that two sets of
// files that collide only in the sample produce separate groups.
let full_hashes: Vec<(i64, Option<u64>)> = entries let full_hashes: Vec<(i64, Option<u64>)> = entries
.par_iter() .par_iter()
.map(|(id, path)| { .map(|(id, path)| {
@@ -981,8 +983,6 @@ pub async fn find_duplicates(
(*id, hash) (*id, hash)
}) })
.collect(); .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>> = let mut by_full: std::collections::HashMap<u64, Vec<i64>> =
std::collections::HashMap::new(); std::collections::HashMap::new();
for (id, maybe_hash) in full_hashes { for (id, maybe_hash) in full_hashes {
@@ -990,20 +990,12 @@ pub async fn find_duplicates(
by_full.entry(h).or_default().push(id); by_full.entry(h).or_default().push(id);
} }
} }
// Return the largest confirmed group (same sample_hash/size bucket). // Emit one entry per distinct full hash that has ≥ 2 members.
// In practice a single full hash will dominate; emit each sub-group. by_full
// We flatten into one entry using the sample_hash key — callers only .into_iter()
// need confirmed IDs, not the exact full hash. .filter(|(_, ids)| ids.len() > 1)
let confirmed_ids: Vec<i64> = by_full .map(|(full_hash, ids)| (full_hash, file_size, ids))
.into_values() .collect()
.filter(|g| g.len() > 1)
.flatten()
.collect();
if confirmed_ids.is_empty() {
None
} else {
Some(((sample_hash, file_size), confirmed_ids))
}
}) })
.collect() .collect()
}) })
@@ -1014,7 +1006,7 @@ pub async fn find_duplicates(
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> = confirmed let mut groups: Vec<DuplicateGroup> = confirmed
.into_iter() .into_iter()
.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 {
file_hash: format!("{:016x}", hash), file_hash: format!("{:016x}", hash),
+32 -4
View File
@@ -511,6 +511,7 @@ pub fn reset_inflight_jobs(conn: &Connection) -> Result<()> {
// Delete any rows that were cancelled before the previous shutdown so // Delete any rows that were cancelled before the previous shutdown so
// they don't silently linger in the DB across restarts. // they don't silently linger in the DB across restarts.
conn.execute("DELETE FROM tagging_jobs WHERE status = 'cancelled'", [])?; conn.execute("DELETE FROM tagging_jobs WHERE status = 'cancelled'", [])?;
conn.execute("DELETE FROM caption_jobs WHERE status = 'cancelled'", [])?;
Ok(()) Ok(())
} }
@@ -588,13 +589,22 @@ pub fn requeue_processing_caption_jobs_for_folder(
} }
pub fn clear_caption_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<usize> { pub fn clear_caption_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<usize> {
// Mirror the tagging worker pattern: mark in-flight rows as cancelled so
// the worker can detect cancellation before writing results, then delete
// every non-processing row immediately (pending, failed, etc.).
match folder_id { match folder_id {
Some(folder_id) => { Some(folder_id) => {
conn.execute(
"UPDATE caption_jobs
SET status = 'cancelled', last_error = NULL, updated_at = datetime('now')
WHERE status = 'processing'
AND image_id IN (SELECT id FROM images WHERE folder_id = ?1)",
[folder_id],
)?;
let deleted = conn.execute( let deleted = conn.execute(
"DELETE FROM caption_jobs "DELETE FROM caption_jobs
WHERE image_id IN ( WHERE status NOT IN ('processing', 'cancelled')
SELECT id FROM images WHERE folder_id = ?1 AND image_id IN (SELECT id FROM images WHERE folder_id = ?1)",
)",
[folder_id], [folder_id],
)?; )?;
conn.execute( conn.execute(
@@ -607,7 +617,16 @@ pub fn clear_caption_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<u
Ok(deleted) Ok(deleted)
} }
None => { None => {
let deleted = conn.execute("DELETE FROM caption_jobs", [])?; conn.execute(
"UPDATE caption_jobs
SET status = 'cancelled', last_error = NULL, updated_at = datetime('now')
WHERE status = 'processing'",
[],
)?;
let deleted = conn.execute(
"DELETE FROM caption_jobs WHERE status NOT IN ('processing', 'cancelled')",
[],
)?;
conn.execute( conn.execute(
"UPDATE images "UPDATE images
SET caption_error = NULL SET caption_error = NULL
@@ -619,6 +638,15 @@ pub fn clear_caption_jobs(conn: &Connection, folder_id: Option<i64>) -> Result<u
} }
} }
pub fn is_caption_job_cancelled(conn: &Connection, image_id: i64) -> Result<bool> {
let count: i64 = conn.query_row(
"SELECT COUNT(*) FROM caption_jobs WHERE image_id = ?1 AND status = 'cancelled'",
[image_id],
|row| row.get(0),
)?;
Ok(count > 0)
}
pub fn reset_generated_captions(conn: &Connection, folder_id: Option<i64>) -> Result<usize> { pub fn reset_generated_captions(conn: &Connection, folder_id: Option<i64>) -> Result<usize> {
let image_ids = match folder_id { let image_ids = match folder_id {
Some(folder_id) => { Some(folder_id) => {
+7
View File
@@ -839,6 +839,13 @@ fn process_caption_batch(
let mut updated_images = Vec::with_capacity(caption_results.len()); let mut updated_images = Vec::with_capacity(caption_results.len());
for (job, caption_result) in &caption_results { for (job, caption_result) in &caption_results {
if db::is_caption_job_cancelled(&tx, job.image_id)? {
tx.execute(
"DELETE FROM caption_jobs WHERE image_id = ?1",
[job.image_id],
)?;
continue;
}
match caption_result { match caption_result {
Ok(caption) => { Ok(caption) => {
updated_images.push(db::update_generated_caption( updated_images.push(db::update_generated_caption(
+9 -2
View File
@@ -253,8 +253,15 @@ pub fn prepare_tagger_model_with_progress(
let local_dir = model_dir(app_data_dir); let local_dir = model_dir(app_data_dir);
std::fs::create_dir_all(&local_dir)?; std::fs::create_dir_all(&local_dir)?;
// Only download the two tagger-specific files; ONNX runtime DLLs are // Ensure the shared ONNX runtime DLLs are present. The tagger shares
// already handled by the captioner download flow. // them with the captioner; install them here so the tagger is fully
// functional even on a clean install where the caption model has never
// been downloaded.
let caption_model_dir = crate::captioner::model_dir(app_data_dir);
std::fs::create_dir_all(&caption_model_dir)?;
crate::captioner::ensure_onnx_runtime(&caption_model_dir)?;
// Download the two tagger-specific files.
const DOWNLOAD_FILES: &[&str] = &["model.onnx", "selected_tags.csv"]; const DOWNLOAD_FILES: &[&str] = &["model.onnx", "selected_tags.csv"];
let api = Api::new()?; let api = Api::new()?;
+27 -1
View File
@@ -238,6 +238,7 @@ interface GalleryState {
similarHasMore: boolean; similarHasMore: boolean;
similarScope: SimilarScope; similarScope: SimilarScope;
similarFolderId: number | null; similarFolderId: number | null;
similarCrop: { x: number; y: number; w: number; h: number } | null;
galleryScrollResetKey: number; galleryScrollResetKey: number;
activeView: ActiveView; activeView: ActiveView;
exploreMode: ExploreMode; exploreMode: ExploreMode;
@@ -567,6 +568,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
similarHasMore: false, similarHasMore: false,
similarScope: "all_media", similarScope: "all_media",
similarFolderId: null, similarFolderId: null,
similarCrop: null,
galleryScrollResetKey: 0, galleryScrollResetKey: 0,
activeView: "gallery", activeView: "gallery",
exploreMode: "visual", exploreMode: "visual",
@@ -771,7 +773,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
}, },
loadMoreImages: async () => { loadMoreImages: async () => {
const { loadedCount, totalImages, loadingImages, collectionTitle, similarSourceImageId, similarHasMore, similarFolderId } = get(); const { loadedCount, totalImages, loadingImages, collectionTitle, similarSourceImageId, similarHasMore, similarFolderId, similarCrop } = get();
if (loadingImages || loadedCount >= totalImages) return; if (loadingImages || loadedCount >= totalImages) return;
if (collectionTitle === "Explore Cluster") return; if (collectionTitle === "Explore Cluster") return;
if (collectionTitle === "Similar Images" && similarSourceImageId !== null) { if (collectionTitle === "Similar Images" && similarSourceImageId !== null) {
@@ -779,6 +781,28 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
await get().loadSimilarImages(similarSourceImageId, similarFolderId, false); await get().loadSimilarImages(similarSourceImageId, similarFolderId, false);
return; return;
} }
if (collectionTitle === "Region Search Results" && similarSourceImageId !== null && similarCrop !== null) {
if (!similarHasMore) return;
const result = await invoke<SimilarImagesPage>("find_similar_by_region", {
params: {
image_id: similarSourceImageId,
crop_x: similarCrop.x,
crop_y: similarCrop.y,
crop_w: similarCrop.w,
crop_h: similarCrop.h,
folder_id: similarFolderId,
offset: loadedCount,
limit: PAGE_SIZE,
},
});
set((state) => ({
images: [...state.images, ...result.images],
loadedCount: state.loadedCount + result.images.length,
totalImages: result.has_more ? state.loadedCount + result.images.length + 1 : state.loadedCount + result.images.length,
similarHasMore: result.has_more,
}));
return;
}
await get().loadImages(false); await get().loadImages(false);
}, },
@@ -1006,6 +1030,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
similarSourceImageId: imageId, similarSourceImageId: imageId,
similarSourceFolderId: sourceFolderId, similarSourceFolderId: sourceFolderId,
similarFolderId: folderId ?? null, similarFolderId: folderId ?? null,
similarCrop: crop,
similarScope, similarScope,
galleryScrollResetKey: state.galleryScrollResetKey + 1, galleryScrollResetKey: state.galleryScrollResetKey + 1,
selectedImage: null, selectedImage: null,
@@ -1038,6 +1063,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
similarSourceFolderId: sourceFolderId, similarSourceFolderId: sourceFolderId,
similarHasMore: result.has_more, similarHasMore: result.has_more,
similarFolderId: folderId ?? null, similarFolderId: folderId ?? null,
similarCrop: crop,
similarScope, similarScope,
}); });
} catch (error) { } catch (error) {