fix: address 5 code review findings (round 5)

- embeddings: move os.close into try/finally so saved_out/saved_err are
  always closed even when os.dup2 restore raises, preventing fd leak
- cache: replace narrow except tuple with except MemoryError: raise /
  except Exception: return None so struct.error and other np.load failures
  return None without masking OOM
- executor: fix first-run advisory message to check effective_count == 0
  (post-stale-cleanup) instead of pre_run_count; remove now-unused
  pre_run_count variable entirely
- jobs: remove dead "skip" entry from strategy_map (unreachable since the
  early-return at the top of _resolve_strategy fires first)
- upload_tracker: log a warning when reset_person encounters a non-list
  flat_key value instead of silently skipping the cleanup

winnow/cache.py

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

winnow/embeddings.py

@@ -42,13 +42,15 @@ def _suppress_output():
                 os.dup2(saved_out, 1)
             except OSError:
                 pass
-            os.close(saved_out)
+            finally:
+                os.close(saved_out)
         if saved_err is not None:
             try:
                 os.dup2(saved_err, 2)
             except OSError:
                 pass
-            os.close(saved_err)
+            finally:
+                os.close(saved_err)
         if devnull_fd is not None:
             os.close(devnull_fd)
 

winnow/executor.py

@@ -327,7 +327,6 @@ def upload_to_frigate(jobs: list[dict]) -> None:
             # TODO(frigate-api): if Frigate exposes per-file embeddings, compute
             # diversity against the full training set (tracked + manual) rather than
             # relying solely on the Frigate score as a proxy signal.
-            pre_run_count = get_tracked_frigate_file_count(name)
             _snapshot = (
                 all_frigate_files.get(name, []) if all_frigate_files is not None
                 else get_frigate_person_files(name)
@@ -360,7 +359,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                     )
             effective_count = get_tracked_frigate_file_count(name)
             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]"
                 )

winnow/jobs.py

@@ -83,7 +83,6 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st
     strategy_map = {
         "adaptive": ("auto", "smart"),
         "auto": ("auto", "smart"),  # legacy alias for adaptive
-        "skip": (0, "skip"),
         "standard": (30, "smart"),
         "broad": (100, "smart"),
     }

winnow/upload_tracker.py

@@ -442,7 +442,12 @@ 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 and isinstance(data[flat_key], list):
+            if 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",
+                    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