fix: pre-empt review findings before next push

- hnsw_index: fetch folder embedding IDs before acquiring the read lock
  in find_similar_image_matches; holding the lock across a SQLite query
  was delaying concurrent ensure_index write-lock requests

- store/DuplicateFinder: add duplicateScanError state; scanDuplicates
  now catches errors and stores them rather than silently leaving the
  user with empty results and no feedback
This commit is contained in:
2026-06-07 23:10:34 +01:00
parent 1f6650021c
commit 7ead26d7fb
3 changed files with 25 additions and 4 deletions
+16 -3
View File
@@ -103,16 +103,29 @@ pub fn find_similar_image_matches(
None => return Ok(Vec::new()), None => return Ok(Vec::new()),
}; };
// Fetch folder image IDs *before* acquiring the read lock so we don't hold
// the lock across a potentially slow SQLite query, which would delay any
// concurrent ensure_index call waiting for a write lock.
let folder_image_ids: Option<Vec<i64>> = if let Some(folder_id) = folder_id {
let ids = vector::get_all_image_embeddings_with_ids(conn, Some(folder_id))?
.into_iter()
.map(|(id, _)| id)
.collect();
Some(ids)
} else {
None
};
let guard = cache().read().expect("hnsw cache poisoned"); let guard = cache().read().expect("hnsw cache poisoned");
let Some(cached) = guard.as_ref() else { let Some(cached) = guard.as_ref() else {
return Ok(Vec::new()); return Ok(Vec::new());
}; };
let knbn = (offset + limit).max(limit).saturating_add(32); let knbn = (offset + limit).max(limit).saturating_add(32);
let neighbours: Vec<Neighbour> = if let Some(folder_id) = folder_id { let neighbours: Vec<Neighbour> = if let Some(image_ids) = folder_image_ids {
let mut allowed_ids = vector::get_all_image_embeddings_with_ids(conn, Some(folder_id))? let mut allowed_ids = image_ids
.into_iter() .into_iter()
.filter_map(|(allowed_image_id, _)| { .filter_map(|allowed_image_id| {
cached.external_by_image_id.get(&allowed_image_id).copied() cached.external_by_image_id.get(&allowed_image_id).copied()
}) })
.collect::<Vec<_>>(); .collect::<Vec<_>>();
+4
View File
@@ -116,6 +116,7 @@ export function DuplicateFinder() {
const duplicateGroups = useGalleryStore((state) => state.duplicateGroups); const duplicateGroups = useGalleryStore((state) => state.duplicateGroups);
const duplicateScanning = useGalleryStore((state) => state.duplicateScanning); const duplicateScanning = useGalleryStore((state) => state.duplicateScanning);
const duplicateScanProgress = useGalleryStore((state) => state.duplicateScanProgress); const duplicateScanProgress = useGalleryStore((state) => state.duplicateScanProgress);
const duplicateScanError = useGalleryStore((state) => state.duplicateScanError);
const duplicateSelectedIds = useGalleryStore((state) => state.duplicateSelectedIds); const duplicateSelectedIds = useGalleryStore((state) => state.duplicateSelectedIds);
const duplicateLastScanned = useGalleryStore((state) => state.duplicateLastScanned); const duplicateLastScanned = useGalleryStore((state) => state.duplicateLastScanned);
const selectedFolderId = useGalleryStore((state) => state.selectedFolderId); const selectedFolderId = useGalleryStore((state) => state.selectedFolderId);
@@ -228,6 +229,9 @@ export function DuplicateFinder() {
</div> </div>
) : null} ) : null}
{duplicateScanError ? (
<p className="mt-2 text-[11px] text-red-400/80">{duplicateScanError}</p>
) : null}
{deleteResult ? ( {deleteResult ? (
<p className="mt-2 text-[11px] text-white/40">{deleteResult}</p> <p className="mt-2 text-[11px] text-white/40">{deleteResult}</p>
) : null} ) : null}
+5 -1
View File
@@ -278,6 +278,7 @@ interface GalleryState {
duplicateGroups: DuplicateGroup[]; duplicateGroups: DuplicateGroup[];
duplicateScanning: boolean; duplicateScanning: boolean;
duplicateScanProgress: { scanned: number; total: number } | null; duplicateScanProgress: { scanned: number; total: number } | null;
duplicateScanError: string | null;
duplicateSelectedIds: Set<number>; duplicateSelectedIds: Set<number>;
duplicateLastScanned: number | null; // Unix timestamp (seconds) duplicateLastScanned: number | null; // Unix timestamp (seconds)
duplicateScanFolderId: number | null | undefined; // undefined = never scanned duplicateScanFolderId: number | null | undefined; // undefined = never scanned
@@ -613,6 +614,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
duplicateGroups: [], duplicateGroups: [],
duplicateScanning: false, duplicateScanning: false,
duplicateScanProgress: null, duplicateScanProgress: null,
duplicateScanError: null,
duplicateSelectedIds: new Set(), duplicateSelectedIds: new Set(),
duplicateLastScanned: null, duplicateLastScanned: null,
duplicateScanFolderId: undefined, duplicateScanFolderId: undefined,
@@ -1466,7 +1468,7 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
scanDuplicates: async (folderId = null) => { scanDuplicates: async (folderId = null) => {
const { listen } = await import("@tauri-apps/api/event"); const { listen } = await import("@tauri-apps/api/event");
set({ duplicateScanning: true, duplicateGroups: [], duplicateScanProgress: null, duplicateSelectedIds: new Set() }); set({ duplicateScanning: true, duplicateGroups: [], duplicateScanProgress: null, duplicateScanError: null, duplicateSelectedIds: new Set() });
const unlisten = await listen<[number, number]>("duplicate_scan_progress", (event) => { const unlisten = await listen<[number, number]>("duplicate_scan_progress", (event) => {
const [scanned, total] = event.payload; const [scanned, total] = event.payload;
set({ duplicateScanProgress: { scanned, total } }); set({ duplicateScanProgress: { scanned, total } });
@@ -1478,6 +1480,8 @@ export const useGalleryStore = create<GalleryState>((set, get) => ({
"Duplicate scan complete", "Duplicate scan complete",
groups.length === 1 ? "Found 1 duplicate group." : `Found ${groups.length.toLocaleString()} duplicate groups.`, groups.length === 1 ? "Found 1 duplicate group." : `Found ${groups.length.toLocaleString()} duplicate groups.`,
); );
} catch (e) {
set({ duplicateScanError: String(e) });
} finally { } finally {
unlisten(); unlisten();
set({ duplicateScanning: false }); set({ duplicateScanning: false });