From 0236ed2d6b2d75c90e5b2bf1fc891f667bfdaf1d Mon Sep 17 00:00:00 2001 From: Holden Date: Tue, 16 Jun 2026 23:51:21 +0000 Subject: [PATCH] fix: address 3 code review findings (round 10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - immich_api: use 'or []' instead of .get("people", []) in get_people so {"people": null} responses (some Immich versions with zero people enrolled) return [] rather than None; .get() default only fires when the key is absent, not when its value is null - embeddings: log OSError from os.dup2 restore at DEBUG rather than silently swallowing it; if a C extension (CUDA/onnxruntime) invalidates the saved fd, the restore fails silently and stdout stays wired to /dev/null — logging makes the event observable without changing the swallow-and-continue semantics - cache: remove MemoryError re-raise from EmbeddingCache.get(); a cache read OOM aborted the entire diversity-selection batch for the person rather than falling back to a fresh embedding computation, which is the more appropriate OOM gate; broadening back to except Exception restores the pre-round-5 fallback behavior --- winnow/cache.py | 2 -- winnow/embeddings.py | 8 ++++---- winnow/immich_api.py | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/winnow/cache.py b/winnow/cache.py index b28f14a..183c3a2 100644 --- a/winnow/cache.py +++ b/winnow/cache.py @@ -75,8 +75,6 @@ class EmbeddingCache: if os.path.exists(path): try: return np.load(path) - except MemoryError: - raise except Exception: return None return None diff --git a/winnow/embeddings.py b/winnow/embeddings.py index 9d59d8a..a715162 100644 --- a/winnow/embeddings.py +++ b/winnow/embeddings.py @@ -45,8 +45,8 @@ def _suppress_output(): if saved_out is not None: try: os.dup2(saved_out, 1) - except OSError: - pass + except OSError as e: + logger.debug("_suppress_output: failed to restore stdout fd: %s", e) finally: try: os.close(saved_out) @@ -55,8 +55,8 @@ def _suppress_output(): if saved_err is not None: try: os.dup2(saved_err, 2) - except OSError: - pass + except OSError as e: + logger.debug("_suppress_output: failed to restore stderr fd: %s", e) finally: try: os.close(saved_err) diff --git a/winnow/immich_api.py b/winnow/immich_api.py index fbe0b3a..6ecc31f 100644 --- a/winnow/immich_api.py +++ b/winnow/immich_api.py @@ -61,7 +61,7 @@ def get_people() -> list[dict]: if not isinstance(data, dict): logger.error("Unexpected response shape from Immich /people: %r", type(data)) return [] - return data.get("people", []) + return data.get("people") or [] except (requests.RequestException, ValueError, AttributeError) as e: logger.error("Failed to fetch people from Immich: %s", e) return []