From eab3d9fe649e83ec0e10188236a2685ca63eb9f0 Mon Sep 17 00:00:00 2001 From: Holden Date: Wed, 17 Jun 2026 01:54:13 +0000 Subject: [PATCH] fix: 3 correctness bugs from codebase audit round 2 - executor.py: clear min_quality_score_for_slot alongside effective_count restore in for/else block; leaving the stale floor from the deleted file's score blocked the next candidate from filling the restored slot - cli.py: guard merge_ids with p.get('id') is not None, consistent with the _smaller_duplicate_ids fix; bare p['id'] raised KeyError on any person dict missing the id field in the auto-merge path - immich_api.py: replace bare data['major'/'minor'/'patch'] subscripts with .get() in get_immich_version; KeyError was silently swallowed by except Exception, causing version-gated flags to disable without warning --- winnow/cli.py | 2 +- winnow/executor.py | 4 ++++ winnow/immich_api.py | 6 +++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/winnow/cli.py b/winnow/cli.py index 242c0be..a4dcf5d 100644 --- a/winnow/cli.py +++ b/winnow/cli.py @@ -118,7 +118,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]: for name, ps in sorted(duplicates.items()): ordered = sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True) survivor = ordered[0] - merge_ids = [p["id"] for p in ordered[1:]] + merge_ids = [p.get("id") for p in ordered[1:] if p.get("id") is not None] rprint( f" [cyan]Merging {name!r} inside Immich:[/cyan] keeping " f"[dim]{survivor['id'][:8]}…[/dim] ({survivor.get('assetCount', 0)} assets), " diff --git a/winnow/executor.py b/winnow/executor.py index e263f4c..e8389f8 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -612,8 +612,12 @@ def upload_to_frigate(jobs: list[dict]) -> None: # All retries exhausted without a successful upload. # Restore the slot freed by the preceding delete so the next # candidate still sees at_cap=True and must beat the replacement gate. + # Also clear the quality floor — the deleted file's score no longer + # represents any live Frigate file, and leaving it blocks the next + # candidate from filling the restored slot. if at_cap: effective_count += 1 + min_quality_score_for_slot = None progress.advance(upload_task) diff --git a/winnow/immich_api.py b/winnow/immich_api.py index 6ecc31f..71ade02 100644 --- a/winnow/immich_api.py +++ b/winnow/immich_api.py @@ -39,7 +39,11 @@ def get_immich_version() -> tuple[int, int, int] | None: ) if resp.ok: data = resp.json() - return (int(data["major"]), int(data["minor"]), int(data["patch"])) + major, minor, patch = data.get("major"), data.get("minor"), data.get("patch") + if major is None or minor is None or patch is None: + logger.debug("Unexpected Immich version schema: %s", data) + return None + return (int(major), int(minor), int(patch)) return None except Exception: return None