From 614542decd24e99dba552c0d175144ac962cec01 Mon Sep 17 00:00:00 2001
From: Holden <holden@arch.fyi>
Date: Wed, 17 Jun 2026 02:20:38 +0000
Subject: [PATCH] fix: 4 findings from codebase audit round 4
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

jobs.py:
- Add p.get("id") guard to valid_people filter in both
  interactive_configure and auto_configure — id-less named persons
  passed through by _handle_duplicate_people are now excluded before
  any bare-subscript access in the configure paths
- Fix bare p["id"] → p.get("id") in the queued-marker check at line 214
  (runs unconditionally on all valid_people during menu display, before
  any user selection or fetch_all_assets guard)

cli.py:
- Remove dead-code survivor_id and merge_ids guards: after the by_name
  fix (line 75 requires p.get("id")), all persons in any ordered list
  have ids, so neither guard can ever fire; removing them prevents
  misleading readers about what states are reachable
---
 winnow/cli.py  | 5 -----
 winnow/jobs.py | 6 +++---
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/winnow/cli.py b/winnow/cli.py
index 440ace9..ff76e87 100644
--- a/winnow/cli.py
+++ b/winnow/cli.py
@@ -119,12 +119,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
         ordered = sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)
         survivor = ordered[0]
         survivor_id = survivor.get("id")
-        if not survivor_id:
-            logger.warning("%r: survivor has no id — skipping merge", name)
-            continue
         merge_ids = [pid for p in ordered[1:] if (pid := p.get("id")) is not None]
-        if not merge_ids:
-            continue
         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/jobs.py b/winnow/jobs.py
index 396fc7a..7bbef1b 100644
--- a/winnow/jobs.py
+++ b/winnow/jobs.py
@@ -198,7 +198,7 @@ def interactive_configure(people: list[dict]) -> list[dict]:
     Supports multi-person batch mode — after configuring one person,
     prompts to add another.
     """
-    valid_people = sorted([p for p in people if p.get("name")], key=lambda x: x["name"])
+    valid_people = sorted([p for p in people if p.get("name") and p.get("id")], key=lambda x: x["name"])
 
     if not valid_people:
         rprint("[red]No people found with names in Immich.[/red]")
@@ -211,7 +211,7 @@ def interactive_configure(people: list[dict]) -> list[dict]:
         console.print("\n[bold cyan]Select Person to Train:[/bold cyan]")
         for idx, p in enumerate(valid_people, 1):
             # Mark already-queued people
-            marker = " [dim](queued)[/dim]" if any(j["person"]["id"] == p["id"] for j in jobs) else ""
+            marker = " [dim](queued)[/dim]" if any(j["person"]["id"] == p.get("id") for j in jobs) else ""
             console.print(f"  [bold]{idx}.[/bold] {p['name']}{marker}")
 
         p_choice = IntPrompt.ask("Enter Number", choices=[str(i) for i in range(1, len(valid_people) + 1)])
@@ -230,7 +230,7 @@ def interactive_configure(people: list[dict]) -> list[dict]:
 
 def auto_configure(people: list[dict]) -> list[dict]:
     """Non-interactive: configure jobs for all named people automatically."""
-    valid_people = sorted([p for p in people if p.get("name")], key=lambda x: x["name"])
+    valid_people = sorted([p for p in people if p.get("name") and p.get("id")], key=lambda x: x["name"])
 
     if not valid_people:
         rprint("[red]No people found with names in Immich.[/red]")