fix: 4 findings from codebase audit round 4
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
This commit is contained in:
@@ -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), "
|
||||
|
||||
+3
-3
@@ -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]")
|
||||
|
||||
Reference in New Issue
Block a user