From 8bdce9253a723fddac99ebc59211204a453caca1 Mon Sep 17 00:00:00 2001 From: Holden Date: Tue, 16 Jun 2026 20:51:14 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20address=2010=20codebase=20audit=20findin?= =?UTF-8?q?gs=20=E2=80=94=20API=20guards,=20reconcile,=20merge=20fallback,?= =?UTF-8?q?=20tracker=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - immich_api: guard resp.json() with isinstance(dict) check in get_people and fetch_all_assets so AttributeError doesn't escape on proxy/CDN non-dict responses - executor: move actually_uploaded.append outside try/else so Frigate filename→asset_id mapping is created via reconcile even when the tracker write fails - cli: fall back to pre-merge people list when re-fetch after merge returns empty (transient error) instead of silently dropping all people - cli: treat ENABLE_FRIGATE_SCORES=false / BLUR_THRESHOLD=0 as not-set in the unsupported-vars warning (falsy string check replaces raw truthiness) - upload_tracker: guard set(data[flat_key]) with isinstance(list) check in reset_person so a corrupted non-iterable legacy field doesn't crash mid-reset - upload_tracker: guard dims[0]/dims[1] in find_by_crop_dimension with a length check so a truncated crop_dims entry doesn't raise IndexError - cache: wrap os.remove() in clear() with try/except OSError to handle TOCTOU race with concurrent put() calls - diversity: default conf_array to 0.5 (was 1.0) for faces with missing confidence so they receive a moderate diversity boost instead of being treated as high-confidence - diversity: sort assets in the fast path (len <= limit) so return order is consistent with the sorted-by-fileCreatedAt path --- winnow/cache.py | 7 +++++-- winnow/cli.py | 9 ++++++++- winnow/diversity.py | 10 ++++++---- winnow/executor.py | 4 +++- winnow/immich_api.py | 14 +++++++++++--- winnow/upload_tracker.py | 4 +++- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/winnow/cache.py b/winnow/cache.py index 6ce3126..183c3a2 100644 --- a/winnow/cache.py +++ b/winnow/cache.py @@ -103,8 +103,11 @@ class EmbeddingCache: count = 0 for f in os.listdir(self.cache_dir): if f.endswith(".npy"): - os.remove(os.path.join(self.cache_dir, f)) - count += 1 + try: + os.remove(os.path.join(self.cache_dir, f)) + count += 1 + except OSError: + pass logger.info("Cleared %s cached embeddings.", count) diff --git a/winnow/cli.py b/winnow/cli.py index 2ce8d60..e6a6549 100644 --- a/winnow/cli.py +++ b/winnow/cli.py @@ -131,6 +131,12 @@ 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: + logger.warning( + "Re-fetch after merge returned no people" + " — possible transient error; proceeding with pre-merge list" + ) + 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 @@ -173,7 +179,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: " diff --git a/winnow/diversity.py b/winnow/diversity.py index 4e9ada2..36ffa47 100644 --- a/winnow/diversity.py +++ b/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", "")) @@ -483,8 +483,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 - conf_array = np.ones(n) + # Build confidence weight array for hard example boosting. + # Default to 0.5 for faces with no confidence score so they receive a + # moderate diversity boost rather than being treated as high-confidence. + conf_array = np.full(n, 0.5) if confidence_scores: for i, c in enumerate(confidence_scores): if c is not None: diff --git a/winnow/executor.py b/winnow/executor.py index 0290da0..ef235b9 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -533,7 +533,9 @@ def upload_to_frigate(jobs: list[dict]) -> None: else: if pre_fscore is not None: person_has_fscores = True - actually_uploaded.append((fname, asset_id)) + # Always record for reconcile so the Frigate filename→asset_id + # mapping is created even when the tracker write fails. + actually_uploaded.append((fname, asset_id)) break else: diff --git a/winnow/immich_api.py b/winnow/immich_api.py index 3bde08b..935b62b 100644 --- a/winnow/immich_api.py +++ b/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", []) + 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): diff --git a/winnow/upload_tracker.py b/winnow/upload_tracker.py index b5b90fc..859b78e 100644 --- a/winnow/upload_tracker.py +++ b/winnow/upload_tracker.py @@ -360,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({ @@ -440,7 +442,7 @@ 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 isinstance(data[flat_key], list): data[flat_key] = sorted(set(data[flat_key]) - person_ids) _save(filename, data) changed = True