fix: address 2 code review findings (round 11)

- upload_tracker: revert data[flat_key] = [] from round 8; clearing the
  entire shared legacy flat list on a corrupt value wipes all persons'
  IDs, not just the one being reset; since a corrupt non-list value is
  already unreadable by load_uploaded_ids, leaving it in place is safer
  than a mass-wipe; update warning message to note the field is unaffected
  but unreadable so the corruption is still observable
- cli: use p.get("id") instead of p["id"] in both people-list fallback
  returns (_handle_duplicate_people lines 144 and 157) for consistency
  with the success path at line 149; bare subscript crashes on malformed
  unnamed persons that bypass _smaller_duplicate_ids
This commit is contained in:
2026-06-17 00:02:49 +00:00
parent 0236ed2d6b
commit 2182c87c40
2 changed files with 4 additions and 4 deletions
+2 -2
View File
@@ -109,7 +109,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
) )
# Return deduplicated list — keep only the largest per name so that # Return deduplicated list — keep only the largest per name so that
# downstream job creation never runs two jobs for the same Frigate folder. # downstream job creation never runs two jobs for the same Frigate folder.
return [p for p in people if p["id"] not in skip_ids] return [p for p in people if p.get("id") not in skip_ids]
# Auto-merge: survivor = largest asset count, rest merge into it inside Immich # Auto-merge: survivor = largest asset count, rest merge into it inside Immich
merged_any = False merged_any = False
@@ -141,7 +141,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
" — possible transient error or expired API key;" " — possible transient error or expired API key;"
" proceeding with pre-merge list. Check IMMICH_API_KEY if this recurs." " proceeding with pre-merge list. Check IMMICH_API_KEY if this recurs."
) )
return [p for p in people if p["id"] not in skip_ids] 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 # 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 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 # IDs from groups that merged successfully are already gone from Immich, so
+2 -2
View File
@@ -444,10 +444,10 @@ def reset_person(person_name: str) -> None:
person_ids = set(_get_ids(tracker_entry)) person_ids = set(_get_ids(tracker_entry))
if person_ids and flat_key in data and not isinstance(data[flat_key], list): if person_ids and flat_key in data and not isinstance(data[flat_key], list):
logger.warning( logger.warning(
"reset_person: %s has unexpected type for %s (%s) — clearing corrupt flat-list", "reset_person: %s has unexpected type for %s (%s) — skipping flat-list cleanup;"
" all persons' legacy IDs in this field are unaffected but unreadable",
filename, flat_key, type(data[flat_key]).__name__, filename, flat_key, type(data[flat_key]).__name__,
) )
data[flat_key] = []
elif person_ids and flat_key in data: elif person_ids and flat_key in data:
data[flat_key] = sorted(set(data[flat_key]) - person_ids) data[flat_key] = sorted(set(data[flat_key]) - person_ids)
_save(filename, data) _save(filename, data)