From 046004a5d0ae17de57feebe57c4440afea4f075d Mon Sep 17 00:00:00 2001 From: Holden Date: Fri, 12 Jun 2026 18:51:44 +0000 Subject: [PATCH] fix: log thumbnail fetch failures and restore get() for duplicate-ID safety Two small correctness fixes from post-commit code review: - except Exception: continue swallowed network/auth errors silently; add logger.debug so systematic failures are diagnosable in winnow.log - batch_images.pop() regressed duplicate-asset-ID handling: if Immich returns the same asset ID twice within the same 32-item batch window (pagination edge case), the second occurrence got None and its embedding was silently dropped. Switching back to .get() matches the old thumbnail_map.get() behaviour. Peak memory is still bounded to _BATCH images because batch_images goes out of scope between batches. Co-Authored-By: Claude Sonnet 4.6 --- winnow/diversity.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/winnow/diversity.py b/winnow/diversity.py index 1e20cc7..0e47144 100644 --- a/winnow/diversity.py +++ b/winnow/diversity.py @@ -227,12 +227,14 @@ def _select_by_embedding( img = future.result() if img is not None: batch_images[asset["id"]] = img - except Exception: + except Exception as e: + logger.debug(f"Failed to fetch thumbnail for {asset['id']}: {e}") continue - # Process and immediately release each image to cap peak memory + # Process each image; batch_images goes out of scope after this loop, + # bounding peak thumbnail memory to _BATCH images per iteration. for asset in batch: - img = batch_images.pop(asset["id"], None) + img = batch_images.get(asset["id"]) processed += 1 if progress_callback: progress_callback(processed, len(candidates))