fix: address 8 code review findings (round 4)

- diversity: remove erroneous break outside if-faces in _scale_bbox_to_thumbnail
  (broke people-loop early for first unannotated person, defeating scale fix)
- diversity: increment quality_filtered for face-too-small crop skips so the
  summary log counts them alongside assess_quality failures
- diversity: fix hard-example log count to use original confidence_scores[i]
  instead of synthetic conf_array default (0.5), eliminating false 100%
  hard-example reports for persons with no Immich confidence data
- cache: add EOFError to except tuple in EmbeddingCache.get() so truncated
  .npy files return None instead of crashing the embedding pipeline
- embeddings: wrap each os.dup2 restore in its own try/except OSError in
  _suppress_output finally block so stderr is always restored even if the
  stdout restore raises
- executor: gate recognize_face on effective_count > 0 (post-stale-cleanup)
  instead of pre_run_count > 0 so recognize_face is not called against an
  untrained Frigate model after the user manually deletes all training files
- executor: document actually_uploaded trade-off in comment (appending
  unconditionally on tracker failure risks a Frigate duplicate but prevents
  permanent filename unmapping which breaks quality-replacement scoring)
- jobs: check strategy == "skip" before the has_embedding and custom_limit
  early-returns in _resolve_strategy so STRATEGY=skip is always honoured

winnow/cache.py

@@ -75,7 +75,7 @@ class EmbeddingCache:
         if os.path.exists(path):
             try:
                 return np.load(path)
-            except (OSError, ValueError):
+            except (OSError, ValueError, EOFError):
                 return None
         return None
 

winnow/diversity.py

@@ -199,7 +199,6 @@ def _scale_bbox_to_thumbnail(
             meta_h = faces[0].get("imageHeight") or img_h
             scale_x, scale_y = img_w / meta_w, img_h / meta_h
             return (x1 * scale_x, y1 * scale_y, x2 * scale_x, y2 * scale_y)
-        break
     return bbox
 
 
@@ -304,6 +303,7 @@ def _select_by_embedding(
                     "Face too small to crop for %s — skipping to avoid embedding wrong person",
                     asset["id"],
                 )
+                quality_filtered += 1
                 continue
             embed_img = face_crop if face_crop is not None else img
 
@@ -581,8 +581,13 @@ def _cluster_aware_selection(
         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

winnow/embeddings.py

@@ -38,10 +38,16 @@ def _suppress_output():
         yield
     finally:
         if saved_out is not None:
+            try:
                 os.dup2(saved_out, 1)
+            except OSError:
+                pass
             os.close(saved_out)
         if saved_err is not None:
+            try:
                 os.dup2(saved_err, 2)
+            except OSError:
+                pass
             os.close(saved_err)
         if devnull_fd is not None:
             os.close(devnull_fd)

winnow/executor.py

@@ -393,7 +393,7 @@ 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.
+                    # skipped when effective_count == 0 since Frigate has no model yet.
                     # 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.
@@ -409,7 +409,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 effective_count > 0:
                         if not at_cap or person_has_fscores:
                             _result = recognize_face(fpath)
                             if _result is not None and (_result[0] or "").casefold() == name.casefold():
@@ -417,8 +417,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:
@@ -535,6 +535,11 @@ def upload_to_frigate(jobs: list[dict]) -> 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, this asset_id is not in
+                                    # the upload set, so it may be re-selected next run (Frigate
+                                    # duplicate). The alternative — not appending — leaves the file
+                                    # permanently unmapped, breaking quality-replacement scoring.
+                                    # Frigate duplicate is the lesser consequence.
                                     actually_uploaded.append((fname, asset_id))
 
                                 break

winnow/jobs.py

@@ -65,6 +65,8 @@ 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:
         limit = _getenv_int("LIMIT", 30)
         if limit <= 0: