fix: 6 findings from codebase audit round 3

cli.py:
- Filter id-less persons from by_name at construction (root fix for all
  bare-subscript crashes downstream — persons with a name but no id are
  excluded from duplicate detection entirely)
- Belt-and-suspenders on warning-path display: p['id'] → p.get('id')
- Extract survivor_id with .get(); skip group if survivor has no id
- Guard merge_ids: skip API call when list is empty after id filtering
- Walrus operator in merge_ids comprehension: p.get("id") called once
  per item instead of twice

executor.py:
- Add cross-reference comment at success-path reset so the for/else
  rollback pairing is explicit for future maintainers
This commit is contained in:
2026-06-17 02:09:16 +00:00
parent eab3d9fe64
commit 3fccf9c8f9
2 changed files with 12 additions and 7 deletions
+11 -5
View File
@@ -72,7 +72,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
by_name: dict[str, list[dict]] = defaultdict(list)
for p in people:
name = (p.get("name") or "").strip()
if name:
if name and p.get("id"):
by_name[name].append(p)
duplicates = {name: ps for name, ps in by_name.items() if len(ps) > 1}
@@ -95,7 +95,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)
entries = ", ".join(
f"[dim]{p['id'][:8]}…[/dim] ({p.get('assetCount', 0)} assets)"
f"[dim]{(p.get('id') or '?')[:8]}…[/dim] ({p.get('assetCount', 0)} assets)"
for p in ordered
)
rprint(f" [yellow]{name}[/yellow] → {len(ps)} people: {entries}")
@@ -118,13 +118,19 @@ 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.get("id") for p in ordered[1:] if p.get("id") is not None]
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), "
f"[dim]{survivor_id[:8]}…[/dim] ({survivor.get('assetCount', 0)} assets), "
f"absorbing {len(merge_ids)} smaller duplicate(s)..."
)
if merge_people(survivor["id"], merge_ids):
if merge_people(survivor_id, merge_ids):
rprint(f" [green]✓ Merged {name!r}[/green]")
merged_any = True
else:
+1 -2
View File
@@ -514,8 +514,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
uploaded += 1
person_uploaded += 1
effective_count += 1
min_quality_score_for_slot = None
min_quality_score_for_slot = None # for/else rollback mirrors this pair
asset_id = asset_map.get(fname)
if asset_id:
try: