From 461ceb7af46c1b2e334c8a56709629a809a57b24 Mon Sep 17 00:00:00 2001
From: Holden <holden@arch.fyi>
Date: Tue, 16 Jun 2026 23:24:11 +0000
Subject: [PATCH] fix: address 5 code review findings (round 8)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- diversity: fix hard_count regression from round 7 — revert to
  'is not None and < 0.85' so only images that actually receive a
  FPS boost (confirmed low confidence) are counted as hard examples;
  None-confidence images use conf_array=1.0 (no boost) and should
  not appear in the hard-example log count
- diversity: fix _scale_bbox_to_thumbnail to use explicit zero-guard
  for imageWidth/imageHeight (meta_w or 0; scale = img_w/meta_w if
  meta_w else 1.0) — mirrors image_processing.py pattern; prevents
  `or img_w` from silently treating imageWidth=0 as missing and
  returning scale=1.0 without surfacing the zero-metadata case
- embeddings: wrap all three os.close calls in _suppress_output
  finally block with try/except OSError: pass so a failed close
  in one branch cannot abort the outer finally and leak devnull_fd
  or the saved_err/saved_out fds
- upload_tracker: clear corrupt flat-list key (data[flat_key] = [])
  after the isinstance warning instead of leaving the corrupt value
  in place — prevents stale IDs persisting across reset_person calls
  and future load_uploaded_ids() from seeing a non-list value
- executor: move 'if pre_fscore is not None: person_has_fscores = True'
  out of the try/except else branch so it fires even when mark_uploaded
  raises; Frigate scores exist once measured regardless of tracker
  write success, and replacement strategy should reflect that
---
 winnow/diversity.py      | 10 ++++++----
 winnow/embeddings.py     | 15 ++++++++++++---
 winnow/executor.py       |  5 ++---
 winnow/upload_tracker.py |  3 ++-
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/winnow/diversity.py b/winnow/diversity.py
index 6961f56..7c308fb 100644
--- a/winnow/diversity.py
+++ b/winnow/diversity.py
@@ -195,9 +195,10 @@ def _scale_bbox_to_thumbnail(
             continue
         faces = person.get("faces", [])
         if faces:
-            meta_w = faces[0].get("imageWidth") or img_w
-            meta_h = faces[0].get("imageHeight") or img_h
-            scale_x, scale_y = img_w / meta_w, img_h / meta_h
+            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
 
@@ -587,7 +588,8 @@ def _cluster_aware_selection(
         1 for i in selected
         if confidence_scores
         and i < len(confidence_scores)
-        and (confidence_scores[i] is None or confidence_scores[i] < 0.85)
+        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)
 
diff --git a/winnow/embeddings.py b/winnow/embeddings.py
index 53f254e..1e84ed4 100644
--- a/winnow/embeddings.py
+++ b/winnow/embeddings.py
@@ -43,16 +43,25 @@ def _suppress_output():
             except OSError:
                 pass
             finally:
-                os.close(saved_out)
+                try:
+                    os.close(saved_out)
+                except OSError:
+                    pass
         if saved_err is not None:
             try:
                 os.dup2(saved_err, 2)
             except OSError:
                 pass
             finally:
-                os.close(saved_err)
+                try:
+                    os.close(saved_err)
+                except OSError:
+                    pass
         if devnull_fd is not None:
-            os.close(devnull_fd)
+            try:
+                os.close(devnull_fd)
+            except OSError:
+                pass
 
 
 # Lazy-loaded singleton
diff --git a/winnow/executor.py b/winnow/executor.py
index 52b7401..65114ad 100644
--- a/winnow/executor.py
+++ b/winnow/executor.py
@@ -534,9 +534,8 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                                             " but asset may be re-selected next run: %s",
                                             fname, tracker_exc,
                                         )
-                                    else:
-                                        if pre_fscore is not None:
-                                            person_has_fscores = True
+                                    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
diff --git a/winnow/upload_tracker.py b/winnow/upload_tracker.py
index 6997b6a..39e57a3 100644
--- a/winnow/upload_tracker.py
+++ b/winnow/upload_tracker.py
@@ -444,9 +444,10 @@ def reset_person(person_name: str) -> None:
             person_ids = set(_get_ids(tracker_entry))
             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",
+                    "reset_person: %s has unexpected type for %s (%s) — clearing corrupt flat-list",
                     filename, flat_key, type(data[flat_key]).__name__,
                 )
+                data[flat_key] = []
             elif person_ids and flat_key in data:
                 data[flat_key] = sorted(set(data[flat_key]) - person_ids)
             _save(filename, data)