From af92fe5fcc32707bf2336d0eefab46c16329ef91 Mon Sep 17 00:00:00 2001 From: Holden Date: Fri, 12 Jun 2026 05:24:35 +0000 Subject: [PATCH] fix: bugs and code quality in embeddings, jobs, diversity, config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - embeddings: initialize ctx_id=-1 before try block so the except handler cannot NameError; move insightface_home out of try for the same reason - embeddings: replace contextlib.redirect_stdout/stderr (Python-level only) with fd-level dup2 suppression — actually silences C extension noise from InsightFace during model loading - jobs: fix frigate_count==0 falling through `or` chain; use explicit `is not None` check so a real zero is not treated as missing data - diversity: thread person_id through select_diverse_assets → _select_by_embedding → _get_face_bbox / _get_face_confidence / _crop_face_from_thumbnail so group-photo assets embed the target person's face rather than whichever person is listed first - config: replace type() hack for ConfigManager with a proper class Co-Authored-By: Claude Sonnet 4.6 --- winnow/config.py | 7 ++++++- winnow/diversity.py | 26 +++++++++++++++++--------- winnow/embeddings.py | 28 ++++++++++++++++++++++------ winnow/jobs.py | 23 +++++++++++++++-------- 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/winnow/config.py b/winnow/config.py index d832b6c..bc787ad 100644 --- a/winnow/config.py +++ b/winnow/config.py @@ -161,7 +161,12 @@ class _ConfigAccessor: Config = _ConfigAccessor() -ConfigManager = type("ConfigManager", (), {"get": staticmethod(lambda: _Config())}) + + +class ConfigManager: + @staticmethod + def get() -> _Config: + return _Config() def get_headers() -> dict[str, str]: diff --git a/winnow/diversity.py b/winnow/diversity.py index 4e53e5b..a1359bc 100644 --- a/winnow/diversity.py +++ b/winnow/diversity.py @@ -29,6 +29,7 @@ def select_diverse_assets( entity_name: str, selection_mode: str = "smart", entity_type: str = "face", + person_id: str | None = None, progress_callback=None, ) -> list: """ @@ -59,7 +60,7 @@ def select_diverse_assets( return _select_time_spread(assets, limit) try: - return _select_by_embedding(assets, limit, entity_type, progress_callback) + return _select_by_embedding(assets, limit, entity_type, person_id, progress_callback) except Exception as e: logger.error(f"Smart Diversity failed: {e}. Falling back to time spread.") return _select_time_spread(assets, limit) @@ -80,9 +81,11 @@ def _fetch_thumbnail(asset_id: str, timeout: int = 10) -> Image.Image | None: return None -def _get_face_bbox(asset: dict) -> tuple[float, float, float, float] | None: - """Extract face bounding box from asset metadata if available.""" +def _get_face_bbox(asset: dict, person_id: str | None = None) -> tuple[float, float, float, float] | None: + """Extract face bounding box from asset metadata for the given person.""" for person in asset.get("people", []): + if person_id and person.get("id") != person_id: + continue faces = person.get("faces", []) if faces: f = faces[0] @@ -95,9 +98,11 @@ def _get_face_bbox(asset: dict) -> tuple[float, float, float, float] | None: return None -def _get_face_confidence(asset: dict) -> float | None: - """Extract face detection confidence from asset metadata if available.""" +def _get_face_confidence(asset: dict, person_id: str | None = None) -> float | None: + """Extract face detection confidence from asset metadata for the given person.""" for person in asset.get("people", []): + if person_id and person.get("id") != person_id: + continue faces = person.get("faces", []) if faces: return faces[0].get("score") or faces[0].get("confidence") @@ -108,6 +113,7 @@ def _crop_face_from_thumbnail( img: Image.Image, asset: dict, margin: float = 0.25, + person_id: str | None = None, ) -> Image.Image | None: """Crop the face region from a thumbnail using Immich bbox metadata. @@ -118,11 +124,12 @@ def _crop_face_from_thumbnail( img: Full preview thumbnail asset: Asset dict with people/faces metadata margin: Extra margin around the bbox (fraction, default 25%) + person_id: If provided, only crop from this person's face data. Returns: Cropped face PIL image, or None if no face metadata available """ - bbox = _get_face_bbox(asset) + bbox = _get_face_bbox(asset, person_id=person_id) if bbox is None: return None @@ -169,6 +176,7 @@ def _select_by_embedding( assets: list, limit: int | str, entity_type: str, + person_id: str | None = None, progress_callback=None, ) -> list: """Select assets using embedding-based cluster-aware FPS. @@ -217,11 +225,11 @@ def _select_by_embedding( if img is None: continue - confidence = _get_face_confidence(asset) + confidence = _get_face_confidence(asset, person_id=person_id) # Quality gate: filter before expensive embedding computation if entity_type == "face": - face_bbox = _get_face_bbox(asset) + face_bbox = _get_face_bbox(asset, person_id=person_id) quality = assess_quality( img, face_bbox=face_bbox, @@ -236,7 +244,7 @@ def _select_by_embedding( continue # Crop the target person's face before embedding - face_crop = _crop_face_from_thumbnail(img, asset) + face_crop = _crop_face_from_thumbnail(img, asset, person_id=person_id) embed_img = face_crop if face_crop is not None else img else: embed_img = img diff --git a/winnow/embeddings.py b/winnow/embeddings.py index 67cf223..6cb598a 100644 --- a/winnow/embeddings.py +++ b/winnow/embeddings.py @@ -6,11 +6,11 @@ Unified embedding interface for faces and objects. - Caching: Disk-based cache avoids recomputation on reruns """ -import contextlib import importlib import logging import os import warnings +from contextlib import contextmanager import cv2 import numpy as np @@ -20,6 +20,24 @@ from .cache import get_cache logger = logging.getLogger(__name__) + +@contextmanager +def _suppress_output(): + """Suppress stdout/stderr at the file-descriptor level, silencing C extension noise.""" + devnull_fd = os.open(os.devnull, os.O_WRONLY) + saved_out, saved_err = os.dup(1), os.dup(2) + try: + os.dup2(devnull_fd, 1) + os.dup2(devnull_fd, 2) + yield + finally: + os.dup2(saved_out, 1) + os.dup2(saved_err, 2) + os.close(devnull_fd) + os.close(saved_out) + os.close(saved_err) + + # Lazy-loaded singletons _insightface_app = None _insightface_loaded = False @@ -70,6 +88,8 @@ def get_insightface_app(): # Preload CUDA/cuDNN DLLs BEFORE any ORT InferenceSession is created _preload_cuda_libs() + ctx_id = -1 + insightface_home = os.environ.get("INSIGHTFACE_HOME", os.path.expanduser("~/.insightface")) try: import onnxruntime as ort from insightface.app import FaceAnalysis @@ -78,7 +98,6 @@ def get_insightface_app(): providers = [p for p in ort.get_available_providers() if p != "TensorrtExecutionProvider"] logger.info(f"Available ONNX providers: {providers}") - # Determine device: 0 for GPU, -1 for CPU gpu_providers = { "CUDAExecutionProvider", "ROCmExecutionProvider", @@ -90,9 +109,7 @@ def get_insightface_app(): device_str = "GPU" if ctx_id >= 0 else "CPU" logger.info(f"Loading InsightFace Buffalo_L on {device_str} (ctx_id={ctx_id})...") - # Suppress C-level output during model loading - with open(os.devnull, "w") as devnull, contextlib.redirect_stdout(devnull), contextlib.redirect_stderr(devnull): - insightface_home = os.environ.get("INSIGHTFACE_HOME", os.path.expanduser("~/.insightface")) + with _suppress_output(): _insightface_app = FaceAnalysis(name="buffalo_l", root=insightface_home, providers=providers) _insightface_app.prepare(ctx_id=ctx_id, det_size=(640, 640)) @@ -103,7 +120,6 @@ def get_insightface_app(): return None except Exception as e: logger.error(f"Failed to load InsightFace: {e}") - # Retry on CPU if GPU failed if ctx_id == 0: logger.warning("Retrying InsightFace on CPU...") try: diff --git a/winnow/jobs.py b/winnow/jobs.py index 55af9df..74d6ddc 100644 --- a/winnow/jobs.py +++ b/winnow/jobs.py @@ -81,7 +81,9 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st return strategy_map.get(strategy, ("auto", "smart")) -def _perform_selection(assets: list, limit: int | str, name: str, selection_mode: str, entity_type: str) -> list: +def _perform_selection( + assets: list, limit: int | str, name: str, selection_mode: str, entity_type: str, person_id: str | None = None +) -> list: """Run diversity selection with progress display.""" if selection_mode == "smart": model_display = "InsightFace (face embeddings)" if entity_type == "face" else "SigLIP (visual embeddings)" @@ -104,6 +106,7 @@ def _perform_selection(assets: list, limit: int | str, name: str, selection_mode name, selection_mode=selection_mode, entity_type=entity_type, + person_id=person_id, progress_callback=lambda c, t: progress.update(task, completed=c, total=t), ) @@ -113,7 +116,9 @@ def _perform_selection(assets: list, limit: int | str, name: str, selection_mode rprint(f"\n[cyan]Using time-spread selection for {limit} images...[/cyan]") with console.status(f"[bold]Selecting {limit} images evenly distributed over time...[/bold]"): - selected = select_diverse_assets(assets, limit, name, selection_mode="time", entity_type=entity_type) + selected = select_diverse_assets( + assets, limit, name, selection_mode="time", entity_type=entity_type, person_id=person_id + ) rprint(f" [green]Selected {len(selected)} images using time spread.[/green]") return selected @@ -167,7 +172,9 @@ def _configure_person(person: dict, people: list[dict]) -> dict | None: return None # Perform selection - selected_assets = _perform_selection(recent_assets, limit, name, selection_mode, entity_type) + selected_assets = _perform_selection( + recent_assets, limit, name, selection_mode, entity_type, person_id=person["id"] + ) rprint(f" [green]Queued {len(selected_assets)} images for {name}.[/green]") return {"person": person, "assets": selected_assets, "limit": len(selected_assets), "config": config} @@ -276,10 +283,8 @@ def auto_configure(people: list[dict]) -> list[dict]: if frigate_counts is not None: already_uploaded = frigate_counts.get(name, 0) else: - already_uploaded = ( - person_summary.get("frigate_count") - or person_summary.get("uploaded", 0) - ) + fc = person_summary.get("frigate_count") + already_uploaded = fc if fc is not None else person_summary.get("uploaded", 0) capacity = Config.MAX_AUTO_IMAGES - already_uploaded if capacity <= 0: rprint( @@ -301,7 +306,9 @@ def auto_configure(people: list[dict]) -> list[dict]: if selection_mode == "skip": continue - selected_assets = _perform_selection(recent_assets, limit, name, selection_mode, entity_type) + selected_assets = _perform_selection( + recent_assets, limit, name, selection_mode, entity_type, person_id=person["id"] + ) if selected_assets: rprint(f" [green]Queued {len(selected_assets)} images for {name}.[/green]")