Merge pull request #35 from sudolulo/dev

release: v0.6.4

CHANGELOG.md

@@ -7,6 +7,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.6.4] - 2026-06-17
+
+### Fixed
+
+- **Face bbox scaled to thumbnail space before quality filtering**: `assess_quality` now receives coordinates in thumbnail-pixel space rather than detection-image space. Previously, a face detected on a full-resolution image (e.g. 4000 px wide) was compared against `MIN_FACE_WIDTH` using its original pixel dimensions, causing faces that appear small on the thumbnail to pass the quality filter — and faces that appear large to be incorrectly rejected.
+
+- **`conf_array` default restored to 1.0 for faces with missing confidence**: the default was incorrectly set to 0.5, causing images with no `score` field in the Immich faces API response to receive a 1.7× FPS diversity boost and be selected ahead of genuinely high-confidence detections. The default is now 1.0 (no boost), treating missing confidence as neutral.
+
+- **`hard_weight` computed once outside FPS loop**: `conf_array` is constant after initialisation; moving the `np.where` call outside the `while` loop eliminates one O(n) numpy pass per selected image.
+
+- **`has_frigate_model` snapshot prevents mid-batch `recognize_face` calls on first run**: `effective_count` is incremented inside the upload loop, so using it as the `recognize_face` gate would incorrectly trigger scoring after the first upload on a first run. A boolean snapshot is now taken before the loop.
+
+- **`person_has_fscores` only set when tracker write succeeds**: the flag was moved outside the `try/except else` block, causing at-cap replacement to switch into Frigate-score mode even when the score was never written to the tracker — `get_most_redundant_mapped_file` then returned `None` and all replacement candidates were silently skipped. The flag is now set only in the `else` branch.
+
+- **`STRATEGY=skip` honoured before embedding and limit checks**: the strategy was silently converted to `auto` when InsightFace was available, because two early-returns in `_resolve_strategy` ran before the `strategy_map` lookup.
+
+- **`limit="auto"` preserved on first run**: switching to `limit = capacity` unconditionally caused the FPS adaptive early-stop to never fire on a person's first upload run. `limit="auto"` is now kept when `already_uploaded == 0`.
+
+- **`EmbeddingCache.get` falls back gracefully on all load errors**: a `MemoryError` during `np.load` of a cached embedding was re-raised, crashing the entire diversity-selection batch for that person. Cache-read failures of any kind now return `None` so the embedding is recomputed fresh.
+
+- **`get_people` returns `[]` when Immich sends `{"people": null}`**: `.get("people", [])` only uses the default when the key is absent, not when its value is `null`. Changed to `data.get("people") or []` so null-valued responses are handled the same as missing keys.
+
+- **`get_people` and `fetch_all_assets` guard against non-dict responses**: a proxy or CDN returning a JSON array (or other non-dict body) previously caused an `AttributeError` from `.get()`. Both functions now check `isinstance(data, dict)` and return an empty result with an error log.
+
+- **`filter_recent_assets` counts and logs assets with missing or unparseable timestamps** instead of silently dropping them.
+
+- **`_suppress_output` fd cleanup restructured**: the context manager now initialises `devnull_fd`, `saved_out`, and `saved_err` to `None` before the `try` block, so the `finally` can close only the descriptors that were successfully opened. Each `os.close` is wrapped in its own `try/except OSError` so a failed close cannot prevent subsequent descriptors from being released. `OSError` from `os.dup2` restore is logged at DEBUG rather than silently swallowed.
+
+- **`blur_score_from_image` copies the image before thumbnail resize**: `Image.thumbnail` modifies the image in-place. When the caller's image was already in RGB mode (no convert copy), the resize would have mutated the caller's object. A copy is now made when `score_img is img`.
+
+- **`imageWidth`/`imageHeight` zero-value treated as missing** in `image_processing.py`: the old `or img_w` fallback silently set `scale = 1.0` for a zero-valued dimension (correct) but also for `None` (also correct) with no distinction. The explicit `scale = img_w / meta_w if meta_w else 1.0` form matches the pattern used in the new `_scale_bbox_to_thumbnail` helper and makes the fallback intent clear.
+
+- **`_mark` and `update_frigate_count` copy before mutate**: both functions now create a shallow copy of the top-level tracker dict before assigning into `by_person`, so a failed `_save` cannot leave the in-memory cache ahead of the on-disk file.
+
+- **`reset_person` flat-list guard only warns when cleanup would have run**: the `isinstance(data[flat_key], list)` check previously emitted a warning even when `person_ids` was empty (a no-op call). The warning is now gated behind `person_ids and`, matching the guard on the cleanup branch.
+
+- **`_handle_duplicate_people` uses `p.get("id")` consistently**: all four return-path filter comprehensions and the `_smaller_duplicate_ids` set comprehension now use `.get("id")` instead of bare `p["id"]`, preventing a `KeyError` if the Immich API returns a person record without an `id` field.
+
+- **`K-Medoids` non-medoid membership test is O(1)**: `non_medoids` now filters against `set(medoids)` instead of the list, eliminating an O(k) scan per candidate on each outer iteration.
+
 ## [0.6.3] - 2026-06-16
 
 ### Fixed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "winnow"
-version = "0.6.3"
+version = "0.6.4"
 description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition."
 license = "AGPL-3.0-or-later"
 requires-python = ">=3.13"

winnow/cache.py

@@ -103,8 +103,11 @@ class EmbeddingCache:
         count = 0
         for f in os.listdir(self.cache_dir):
             if f.endswith(".npy"):
+                try:
                     os.remove(os.path.join(self.cache_dir, f))
                     count += 1
+                except OSError:
+                    pass
         logger.info("Cleared %s cached embeddings.", count)
 
 

winnow/cli.py

@@ -81,7 +81,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
     def _smaller_duplicate_ids(groups: dict) -> set[str]:
         """IDs of all but the largest person in each duplicate group."""
         return {
-            p["id"]
+            p.get("id")
             for ps in groups.values()
             for p in sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)[1:]
         }
@@ -109,7 +109,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
         )
         # Return deduplicated list — keep only the largest per name so that
         # downstream job creation never runs two jobs for the same Frigate folder.
-        return [p for p in people if p["id"] not in skip_ids]
+        return [p for p in people if p.get("id") not in skip_ids]
 
     # Auto-merge: survivor = largest asset count, rest merge into it inside Immich
     merged_any = False
@@ -131,6 +131,17 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
     if merged_any:
         rprint("  [dim]Re-fetching people after merge...[/dim]")
         fresh = get_people()
+        if not fresh:
+            # Retry once: get_people() returns [] for both transient failures and
+            # auth errors (401); a second empty result strongly suggests a real failure.
+            fresh = get_people()
+        if not fresh:
+            logger.warning(
+                "Re-fetch after merge returned no people (tried twice)"
+                " — possible transient error or expired API key;"
+                " proceeding with pre-merge list. Check IMMICH_API_KEY if this recurs."
+            )
+            return [p for p in people if p.get("id") not in skip_ids]
         # Filter out the smaller duplicate from any group whose merge failed — those
         # IDs still exist in Immich and would produce two jobs for the same folder.
         # IDs from groups that merged successfully are already gone from Immich, so
@@ -143,7 +154,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
         "  [yellow]All merges failed — applying local deduplication"
         " to avoid overwriting output.[/yellow]"
     )
-    return [p for p in people if p["id"] not in skip_ids]
+    return [p for p in people if p.get("id") not in skip_ids]
 
 
 _UNSUPPORTED_VARS = [
@@ -173,7 +184,8 @@ def main() -> None:
     [dim]Immich -> Frigate Training Data Curator[/dim]
         """)
 
-        set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v)]
+        _FALSY = {"", "false", "0", "no", "off"}
+        set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v, "").strip().lower() not in _FALSY]
         if set_unsupported:
             console.print(
                 f"[bold yellow]⚠  Advanced tuning vars set: "

winnow/config.py

@@ -173,7 +173,7 @@ class _Config:
                 data = json.loads(config_file.read_text())
                 if not self.IMMICH_URL:
                     self.IMMICH_URL = data.get("IMMICH_URL")
-                if os.getenv("OUTPUT_DIR") is None:
+                if not os.getenv("OUTPUT_DIR"):
                     self.OUTPUT_DIR = data.get("OUTPUT_DIR", self.OUTPUT_DIR)
             except (json.JSONDecodeError, OSError) as e:
                 logging.warning("Failed to load config file: %s", e)

winnow/diversity.py

@@ -57,9 +57,9 @@ def select_diverse_assets(
     Returns:
         List of selected assets
     """
-    # Fast path: fewer assets than limit
+    # Fast path: fewer assets than limit — sort for consistent ordering with other paths
     if limit != "auto" and len(assets) <= limit:
-        return assets
+        return sorted(assets, key=lambda x: x.get("fileCreatedAt", ""))
 
     # Sort by creation time
     assets = sorted(assets, key=lambda x: x.get("fileCreatedAt", ""))
@@ -181,6 +181,28 @@ def _crop_face_from_thumbnail(
     return crop
 
 
+def _scale_bbox_to_thumbnail(
+    bbox: tuple[float, float, float, float],
+    img: Image.Image,
+    asset: dict,
+    person_id: str | None = None,
+) -> tuple[float, float, float, float]:
+    """Scale a face bbox from original detection-image space to thumbnail-pixel space."""
+    x1, y1, x2, y2 = bbox
+    img_w, img_h = img.size
+    for person in asset.get("people", []):
+        if person_id and person.get("id") != person_id:
+            continue
+        faces = person.get("faces", [])
+        if faces:
+            meta_w = faces[0].get("imageWidth") or 0
+            meta_h = faces[0].get("imageHeight") or 0
+            scale_x = img_w / meta_w if meta_w else 1.0
+            scale_y = img_h / meta_h if meta_h else 1.0
+            return (x1 * scale_x, y1 * scale_y, x2 * scale_x, y2 * scale_y)
+    return bbox
+
+
 # =============================================================================
 # Embedding Collection
 # =============================================================================
@@ -258,9 +280,13 @@ def _select_by_embedding(
             confidence = _get_face_confidence(asset, person_id=person_id)
 
             face_bbox = _get_face_bbox(asset, person_id=person_id)
+            thumbnail_bbox = (
+                _scale_bbox_to_thumbnail(face_bbox, img, asset, person_id)
+                if face_bbox is not None else None
+            )
             quality = assess_quality(
                 img,
-                face_bbox=face_bbox,
+                face_bbox=thumbnail_bbox,
                 confidence=confidence,
                 blur_threshold=Config.BLUR_THRESHOLD,
                 min_face_px=Config.MIN_FACE_WIDTH,
@@ -408,7 +434,8 @@ def _kmedoids(dist_matrix: np.ndarray, k: int, max_iter: int = 50) -> tuple[list
     for _ in range(max_iter):
         improved = False
         # Try swapping each medoid with a random non-medoid
-        non_medoids = [i for i in range(n) if i not in medoids]
+        medoid_set = set(medoids)
+        non_medoids = [i for i in range(n) if i not in medoid_set]
         if not non_medoids:
             break
 
@@ -483,7 +510,10 @@ def _cluster_aware_selection(
     norms = np.linalg.norm(emb_matrix, axis=1, keepdims=True)
     emb_normed = emb_matrix / np.maximum(norms, 1e-8)
 
-    # Build confidence weight array for hard example boosting
+    # Build confidence weight array for hard example boosting.
+    # Default to 1.0 for faces with no confidence score: treat as high-confidence
+    # (no boost) rather than hard-example territory. A missing score field should
+    # not cause these images to beat genuinely high-confidence detections in FPS.
     conf_array = np.ones(n)
     if confidence_scores:
         for i, c in enumerate(confidence_scores):
@@ -508,7 +538,6 @@ def _cluster_aware_selection(
 
     medoid_indices, cluster_labels = _kmedoids(dist_matrix, k)
     selected = list(medoid_indices)
-    selected_set = set(selected)
 
     logger.debug("Selected %s cluster medoids as initial picks.", len(selected))
 
@@ -522,10 +551,11 @@ def _cluster_aware_selection(
     for idx in selected:
         min_dists[idx] = -np.inf
 
-    while len(selected) < target:
-        # Hard example weighting: boost distance for low-confidence candidates
-        # Confidence < 0.85 gets up to 1.5× distance boost
+    # Hard example weighting: boost distance for low-confidence candidates.
+    # conf_array is constant after this point, so compute once outside the loop.
     hard_weight = np.where(conf_array < 0.85, 1.0 + (0.85 - conf_array) * 2.0, 1.0)
+
+    while len(selected) < target:
         weighted_dists = min_dists * hard_weight
 
         best_idx = int(np.argmax(weighted_dists))
@@ -541,15 +571,19 @@ def _cluster_aware_selection(
             break
 
         selected.append(best_idx)
-        selected_set.add(best_idx)
 
         # Update min distances
         dists_to_new = dist_matrix[best_idx]
         min_dists = np.minimum(min_dists, dists_to_new)
         min_dists[best_idx] = -np.inf
 
-    selected_conf = [conf_array[i] for i in selected if conf_array[i] < 1.0]
-    hard_count = sum(1 for c in selected_conf if c < 0.85)
+    hard_count = sum(
+        1 for i in selected
+        if confidence_scores
+        and i < len(confidence_scores)
+        and confidence_scores[i] is not None
+        and confidence_scores[i] < 0.85
+    )
     logger.info("Selection complete: %s images (%s hard examples with confidence < 0.85).", len(selected), hard_count)
 
     # Slice to target: the while loop enforces this for non-auto mode, but
@@ -576,4 +610,4 @@ def _select_time_spread(assets: list, limit: int | str) -> list:
         return assets
 
     indices = np.linspace(0, len(assets) - 1, limit, dtype=int)
-    return [assets[i] for i in np.unique(indices)]
+    return [assets[i] for i in indices]

winnow/embeddings.py

@@ -26,22 +26,47 @@ 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)
+    devnull_fd = None
+    saved_out = None
+    saved_err = None
     try:
+        devnull_fd = os.open(os.devnull, os.O_WRONLY)
+        saved_out = os.dup(1)
+        saved_err = os.dup(2)
         os.dup2(devnull_fd, 1)
         os.dup2(devnull_fd, 2)
         yield
     finally:
+        # Each block is a separate sequential statement. A BaseException (e.g.
+        # KeyboardInterrupt) raised inside block N would propagate past blocks N+1
+        # and N+2, leaving saved_err or devnull_fd unclosed. In CPython, KI is
+        # delivered between bytecodes, not mid-syscall; os.dup2 is a single C call
+        # and completes atomically, so this race is not realistically triggerable.
+        if saved_out is not None:
             try:
                 os.dup2(saved_out, 1)
+            except OSError as e:
+                logger.debug("_suppress_output: failed to restore stdout fd: %s", e)
             finally:
                 try:
-                os.dup2(saved_err, 2)
-            finally:
-                os.close(devnull_fd)
                     os.close(saved_out)
+                except OSError:
+                    pass
+        if saved_err is not None:
+            try:
+                os.dup2(saved_err, 2)
+            except OSError as e:
+                logger.debug("_suppress_output: failed to restore stderr fd: %s", e)
+            finally:
+                try:
                     os.close(saved_err)
+                except OSError:
+                    pass
+        if devnull_fd is not None:
+            try:
+                os.close(devnull_fd)
+            except OSError:
+                pass
 
 
 # Lazy-loaded singleton
@@ -181,8 +206,9 @@ def get_face_embedding(img_pil: Image.Image) -> np.ndarray | None:
         return None
 
     try:
-        # InsightFace expects BGR cv2 image
-        img_bgr = cv2.cvtColor(np.asarray(img_pil), cv2.COLOR_RGB2BGR)
+        # InsightFace expects BGR cv2 image; normalise mode first so RGBA/grayscale don't
+        # raise a channel-count error inside cvtColor.
+        img_bgr = cv2.cvtColor(np.asarray(img_pil.convert("RGB")), cv2.COLOR_RGB2BGR)
 
         # Suppress scikit-image FutureWarning from InsightFace's face_align.py
         with warnings.catch_warnings():

winnow/executor.py

@@ -1,6 +1,7 @@
 """Execution phase: image processing and Frigate upload."""
 
 import logging
+import operator
 import os
 import shutil
 from io import BytesIO
@@ -27,8 +28,10 @@ from .log_config import console
 from .quality import blur_score_from_image
 from .reconcile import enrich_asset_with_face_data, reconcile_frigate_mappings
 from .upload_tracker import (
-    UPLOAD_TRACKER_FILE,
     REJECT_TRACKER_FILE,
+    UPLOAD_TRACKER_FILE,
+    begin_batch,
+    flush_batch,
     get_lowest_quality_mapped_file,
     get_most_redundant_mapped_file,
     get_tracked_frigate_file_count,
@@ -36,8 +39,6 @@ from .upload_tracker import (
     has_frigate_scores,
     mark_rejected,
     mark_uploaded,
-    begin_batch,
-    flush_batch,
     remove_frigate_file,
     remove_frigate_files_batch,
 )
@@ -357,12 +358,16 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                         " (file(s) no longer in Frigate)[/dim]"
                     )
             effective_count = get_tracked_frigate_file_count(name)
-            pre_run_count = effective_count
             quality_replacement = job.get("config", {}).get("quality_replacement", False)
-            if Config.ENABLE_FRIGATE_SCORES and pre_run_count == 0:
+            if Config.ENABLE_FRIGATE_SCORES and effective_count == 0:
                 progress.console.print(
                     f"  [dim]{name}: first run — Frigate diversity scoring will apply from the next run[/dim]"
                 )
+            # Snapshot whether Frigate has a model before the upload loop starts.
+            # effective_count is incremented inside the loop on each successful upload,
+            # so using the live value would incorrectly trigger recognize_face calls
+            # mid-batch on the first run (after the first upload sets it to 1).
+            has_frigate_model = effective_count > 0
             actually_uploaded: list[tuple[str, str | None]] = []
             failed_deletes: set[str] = set()
             min_quality_score_for_slot: float | None = None
@@ -391,8 +396,8 @@ def upload_to_frigate(jobs: list[dict]) -> None:
 
                     # Pre-upload Frigate score — clean measurement (image not yet in training set).
                     # Called for all below-cap uploads (seeds frigate_scores for future at-cap
-                    # replacement) and for at-cap uploads when scores already exist. Skipped on
-                    # the first run (pre_run_count == 0) since Frigate has no model yet.
+                    # replacement) and for at-cap uploads when scores already exist.
+                    # Skipped when has_frigate_model is False (effective_count was 0 before the loop).
                     # recognize_face returns (face_name, score); we only use the score when the
                     # best match is for the correct person. Mismatches (or "unknown") are treated
                     # as None so a wrong-person score never drives a ceiling skip or replacement.
@@ -408,7 +413,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                     # rebuild-complete signal, poll it between recognize calls during replacement
                     # sequences rather than accepting stale/None scores.
                     pre_fscore: float | None = None
-                    if Config.ENABLE_FRIGATE_SCORES and pre_run_count > 0:
+                    if Config.ENABLE_FRIGATE_SCORES and has_frigate_model:
                         if not at_cap or person_has_fscores:
                             _result = recognize_face(fpath)
                             if _result is not None and (_result[0] or "").casefold() == name.casefold():
@@ -416,8 +421,8 @@ def upload_to_frigate(jobs: list[dict]) -> None:
 
                     # Below-cap novelty gate: skip candidates already covered by the Frigate model,
                     # including conditions learned from manually-added images winnow can't track.
-                    # pre_fscore is None on the first run (pre_run_count == 0 skips recognize_face
-                    # above), so this block never fires on the first run without an extra guard.
+                    # pre_fscore is None when effective_count == 0 (no Frigate model yet),
+                    # so this block never fires on the first run without an extra guard.
                     if not at_cap and pre_fscore is not None:
                         _ceiling = Config.FRIGATE_SCORE_CEILING
                         if _ceiling is None:
@@ -452,13 +457,13 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                             get_target = get_most_redundant_mapped_file
                             score_label, better_note = "frigate", " (more novel)"
                             no_score_msg = "Frigate recognize unavailable, skipping replacement"
-                            is_better_than = lambda c, t: c < t
+                            is_better_than = operator.lt
                         else:
                             candidate_score = score_map.get(fname)
                             get_target = get_lowest_quality_mapped_file
                             score_label, better_note = "blur", ""
                             no_score_msg = "no quality score, skipping replacement"
-                            is_better_than = lambda c, t: c > t
+                            is_better_than = operator.gt
 
                         if candidate_score is None:
                             progress.console.print(f"    [dim]⏭  {fname}: {no_score_msg}[/dim]")
@@ -489,7 +494,10 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                             effective_count -= 1
                             min_quality_score_for_slot = None if using_fscore else target_score
                         else:
-                            logger.warning("Failed to delete %s for %s, skipping replacement", target_frigate_file, name)
+                            logger.warning(
+                                "Failed to delete %s for %s, skipping replacement",
+                                target_frigate_file, name,
+                            )
                             failed_deletes.add(target_frigate_file)
                             progress.advance(upload_task)
                             continue
@@ -529,6 +537,16 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                                     else:
                                         if pre_fscore is not None:
                                             person_has_fscores = True
+                                    # Always record for reconcile so the Frigate filename→asset_id
+                                    # mapping is created even when the tracker write fails.
+                                    # Trade-off: if mark_uploaded failed, asset_id is absent from
+                                    # asset_ids and scores. Consequences: (1) re-selected next run
+                                    # → Frigate duplicate; (2) excluded from quality-replacement
+                                    # candidates (_pick_mapped_file requires a scores entry);
+                                    # (3) counted toward MAX_AUTO_IMAGES cap (via frigate_files).
+                                    # The alternative — not appending — leaves the file permanently
+                                    # unmapped (reconcile never creates the frigate_files entry),
+                                    # making (2) and (3) permanent. Frigate duplicate is lesser.
                                     actually_uploaded.append((fname, asset_id))
 
                                 break
@@ -603,11 +621,19 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                 try:
                     flush_batch(UPLOAD_TRACKER_FILE)
                 except Exception as _flush_exc:
-                    logger.warning("flush_batch failed during cleanup — batch will be recovered on next begin_batch: %s", _flush_exc)
+                    logger.warning(
+                        "flush_batch failed during cleanup"
+                        " — batch will be recovered on next begin_batch: %s",
+                        _flush_exc,
+                    )
                 try:
                     flush_batch(REJECT_TRACKER_FILE)
                 except Exception as _flush_exc:
-                    logger.warning("flush_batch failed during cleanup — batch will be recovered on next begin_batch: %s", _flush_exc)
+                    logger.warning(
+                        "flush_batch failed during cleanup"
+                        " — batch will be recovered on next begin_batch: %s",
+                        _flush_exc,
+                    )
 
             # Batch-map Frigate filenames to asset IDs now that all uploads are done.
             if actually_uploaded and not _skip_reconcile:

winnow/image_processing.py

@@ -92,11 +92,14 @@ def process_face_mode(
         return None
 
     img_w, img_h = img.size
-    meta_w = face_info.get("imageWidth") or img_w
-    meta_h = face_info.get("imageHeight") or img_h
+    meta_w = face_info.get("imageWidth") or 0
+    meta_h = face_info.get("imageHeight") or 0
 
-    # Scale bounding box to actual image dimensions
-    scale_x, scale_y = img_w / meta_w, img_h / meta_h
+    # Scale bounding box from detection-image space to actual image dimensions.
+    # Fall back to 1.0 if Immich omits the field — bbox is assumed to already
+    # be in image space (correct for thumbnails, wrong for full-res).
+    scale_x = img_w / meta_w if meta_w else 1.0
+    scale_y = img_h / meta_h if meta_h else 1.0
     x1 = face_info["boundingBoxX1"] * scale_x
     y1 = face_info["boundingBoxY1"] * scale_y
     x2 = face_info["boundingBoxX2"] * scale_x

winnow/immich_api.py

@@ -57,8 +57,12 @@ def get_people() -> list[dict]:
             logger.error("Immich API key is invalid or expired (401 Unauthorized). Update API_KEY.")
             return []
         resp.raise_for_status()
-        return resp.json().get("people", [])
-    except (requests.RequestException, ValueError) as e:
+        data = resp.json()
+        if not isinstance(data, dict):
+            logger.error("Unexpected response shape from Immich /people: %r", type(data))
+            return []
+        return data.get("people") or []
+    except (requests.RequestException, ValueError, AttributeError) as e:
         logger.error("Failed to fetch people from Immich: %s", e)
         return []
 
@@ -118,7 +122,11 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
                 logger.error("Error fetching assets for %s (page %s): %s", name, page, resp.status_code)
                 break
 
-            page_assets = resp.json().get("assets", [])
+            body = resp.json()
+            if not isinstance(body, dict):
+                logger.error("Unexpected response shape fetching assets for %s (page %s): %r", name, page, type(body))
+                break
+            page_assets = body.get("assets", [])
             # Immich ≥2.x returns {"assets": {"items": [...]}};
             # earlier versions returned {"assets": [...]} directly.
             if isinstance(page_assets, dict):
@@ -277,10 +285,11 @@ def filter_recent_assets(assets: list[dict], years: int | None = None) -> list[d
 
     logger.debug("Filtering assets older than %s years (%s)", years, cutoff)
 
-    recent, skipped = [], 0
+    recent, skipped, bad_timestamp = [], 0, 0
     for asset in assets:
         created_at_str = asset.get("fileCreatedAt")
         if not isinstance(created_at_str, str) or not created_at_str:
+            bad_timestamp += 1
             continue
 
         try:
@@ -291,8 +300,14 @@ def filter_recent_assets(assets: list[dict], years: int | None = None) -> list[d
             else:
                 skipped += 1
         except ValueError:
+            bad_timestamp += 1
             continue
 
+    if bad_timestamp:
+        logger.warning(
+            "filter_recent_assets: %s asset(s) had missing or unparseable fileCreatedAt"
+            " and were excluded from the pool.", bad_timestamp
+        )
     logger.debug("Retained %s assets (filtered %s old assets).", len(recent), skipped)
     return recent
 

winnow/jobs.py

@@ -65,8 +65,14 @@ def _get_strategy_choice(has_embedding: bool) -> tuple[int | str, str]:
 
 def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, str]:
     """Resolve env var strategy to (limit, selection_mode) without prompts."""
+    if strategy == "skip":
+        return 0, "skip"
     if not has_embedding:
-        return _getenv_int("LIMIT", 30), "time"
+        limit = _getenv_int("LIMIT", 30)
+        if limit <= 0:
+            logger.warning("LIMIT=%s is invalid — ignoring and using default 30", limit)
+            limit = 30
+        return limit, "time"
 
     custom_limit = _getenv_optional_int("LIMIT")
     if custom_limit is not None:
@@ -295,10 +301,11 @@ def auto_configure(people: list[dict]) -> list[dict]:
         # decides per-image whether to swap; any candidate could be an improvement).
         if not quality_replacement_only:
             if limit == "auto":
-                # Switch from open-ended auto to a fixed budget at remaining capacity
-                # so the diversity selector itself stops at the right count instead of
-                # selecting MAX_AUTO_IMAGES and then discarding the excess by position.
                 if already_uploaded > 0:
+                    # Switch from open-ended auto to a fixed budget at remaining capacity
+                    # so the diversity selector stops at the right count instead of
+                    # selecting more than MAX_AUTO_IMAGES and overflowing the cap.
+                    # First runs keep limit="auto" so FPS adaptive early-stop can fire.
                     limit = capacity
             else:
                 limit = min(limit, capacity)

winnow/quality.py

@@ -155,6 +155,7 @@ def blur_score_from_image(img: Image.Image, max_dim: int = 1440) -> float | None
     try:
         score_img = img.convert("RGB") if img.mode != "RGB" else img
         if score_img.width > max_dim or score_img.height > max_dim:
+            if score_img is img:
                 score_img = score_img.copy()
             score_img.thumbnail((max_dim, max_dim), Image.LANCZOS)
         return _laplacian_var(np.array(score_img))

winnow/reconcile.py

@@ -61,7 +61,7 @@ def reconcile_frigate_mappings(
             try:
                 return float(fname.rsplit("_", 1)[-1].rsplit(".", 1)[0])
             except (ValueError, IndexError):
-                return 0.0
+                return float("inf")
 
         logger.debug(
             "%s: mapping %s file(s) by filename timestamp — assumes Frigate processes"

winnow/upload_tracker.py

@@ -109,7 +109,11 @@ def begin_batch(filename: str) -> None:
         try:
             _write_to_disk(path, _cache[key])
         except Exception:
-            logger.warning("begin_batch: could not flush leftover deferred state for %s — partial progress may be lost", path)
+            logger.warning(
+                "begin_batch: could not flush leftover deferred state for %s"
+                " — partial progress may be lost",
+                path,
+            )
         _deferred.discard(key)
         _dirty.discard(key)
     _deferred.add(key)
@@ -162,7 +166,7 @@ def _mark(
         logger.warning("_mark called with empty person_name for asset %s — asset not recorded", asset_id)
         return
     data = _load(filename)
-    by_person = data.setdefault("by_person", {})
+    by_person = dict(data.get("by_person", {}))
     entry = _migrate_entry(by_person.get(person_name, {}))
     ids = set(entry["asset_ids"])
     ids.add(asset_id)
@@ -174,7 +178,9 @@ def _mark(
     if frigate_score is not None:
         entry["frigate_scores"][asset_id] = round(frigate_score, 4)
     by_person[person_name] = entry
-    _save(filename, data)
+    new_data = dict(data)
+    new_data["by_person"] = by_person
+    _save(filename, new_data)
     logger.debug("Marked %s in %s (%s)", asset_id, filename, person_name)
 
 
@@ -354,6 +360,8 @@ def find_by_crop_dimension(size: int) -> list[dict]:
             asset_to_frigate.setdefault(aid, fn)  # first-seen wins; plain inversion silently drops duplicates
         frigate_scores = entry.get("frigate_scores", {})
         for asset_id, dims in entry.get("crop_dims", {}).items():
+            if not isinstance(dims, (list, tuple)) or len(dims) < 2:
+                continue
             w, h = dims[0], dims[1]
             if w == size or h == size:
                 results.append({
@@ -371,11 +379,13 @@ def find_by_crop_dimension(size: int) -> list[dict]:
 def update_frigate_count(person_name: str, count: int) -> None:
     """Record Frigate's authoritative training image count for a person."""
     data = _load(UPLOAD_TRACKER_FILE)
-    by_person = data.setdefault("by_person", {})
+    by_person = dict(data.get("by_person", {}))
     entry = _migrate_entry(by_person.get(person_name, {}))
     entry["frigate_count"] = count
     by_person[person_name] = entry
-    _save(UPLOAD_TRACKER_FILE, data)
+    new_data = dict(data)
+    new_data["by_person"] = by_person
+    _save(UPLOAD_TRACKER_FILE, new_data)
 
 
 def reset_all_people() -> None:
@@ -432,7 +442,13 @@ def reset_person(person_name: str) -> None:
             data["by_person"] = by_person
             flat_key = _flat_key(filename)
             person_ids = set(_get_ids(tracker_entry))
-            if person_ids and flat_key in data:
+            if person_ids and flat_key in data and not isinstance(data[flat_key], list):
+                logger.warning(
+                    "reset_person: %s has unexpected type for %s (%s) — skipping flat-list cleanup;"
+                    " all persons' legacy IDs in this field are unaffected but unreadable",
+                    filename, flat_key, type(data[flat_key]).__name__,
+                )
+            elif person_ids and flat_key in data:
                 data[flat_key] = sorted(set(data[flat_key]) - person_ids)
             _save(filename, data)
             changed = True