fix: address 10 codebase audit findings — API guards, reconcile, merge fallback, tracker guards
- immich_api: guard resp.json() with isinstance(dict) check in get_people and fetch_all_assets so AttributeError doesn't escape on proxy/CDN non-dict responses - executor: move actually_uploaded.append outside try/else so Frigate filename→asset_id mapping is created via reconcile even when the tracker write fails - cli: fall back to pre-merge people list when re-fetch after merge returns empty (transient error) instead of silently dropping all people - cli: treat ENABLE_FRIGATE_SCORES=false / BLUR_THRESHOLD=0 as not-set in the unsupported-vars warning (falsy string check replaces raw truthiness) - upload_tracker: guard set(data[flat_key]) with isinstance(list) check in reset_person so a corrupted non-iterable legacy field doesn't crash mid-reset - upload_tracker: guard dims[0]/dims[1] in find_by_crop_dimension with a length check so a truncated crop_dims entry doesn't raise IndexError - cache: wrap os.remove() in clear() with try/except OSError to handle TOCTOU race with concurrent put() calls - diversity: default conf_array to 0.5 (was 1.0) for faces with missing confidence so they receive a moderate diversity boost instead of being treated as high-confidence - diversity: sort assets in the fast path (len <= limit) so return order is consistent with the sorted-by-fileCreatedAt path
This commit is contained in:
@@ -103,8 +103,11 @@ class EmbeddingCache:
|
|||||||
count = 0
|
count = 0
|
||||||
for f in os.listdir(self.cache_dir):
|
for f in os.listdir(self.cache_dir):
|
||||||
if f.endswith(".npy"):
|
if f.endswith(".npy"):
|
||||||
|
try:
|
||||||
os.remove(os.path.join(self.cache_dir, f))
|
os.remove(os.path.join(self.cache_dir, f))
|
||||||
count += 1
|
count += 1
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
logger.info("Cleared %s cached embeddings.", count)
|
logger.info("Cleared %s cached embeddings.", count)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+8
-1
@@ -131,6 +131,12 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
|
|||||||
if merged_any:
|
if merged_any:
|
||||||
rprint(" [dim]Re-fetching people after merge...[/dim]")
|
rprint(" [dim]Re-fetching people after merge...[/dim]")
|
||||||
fresh = get_people()
|
fresh = get_people()
|
||||||
|
if not fresh:
|
||||||
|
logger.warning(
|
||||||
|
"Re-fetch after merge returned no people"
|
||||||
|
" — possible transient error; proceeding with pre-merge list"
|
||||||
|
)
|
||||||
|
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
|
||||||
@@ -173,7 +179,8 @@ def main() -> None:
|
|||||||
[dim]Immich -> Frigate Training Data Curator[/dim]
|
[dim]Immich -> Frigate Training Data Curator[/dim]
|
||||||
""")
|
""")
|
||||||
|
|
||||||
set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v)]
|
_FALSY = {"", "false", "0", "no", "off"}
|
||||||
|
set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v, "").strip().lower() not in _FALSY]
|
||||||
if set_unsupported:
|
if set_unsupported:
|
||||||
console.print(
|
console.print(
|
||||||
f"[bold yellow]⚠ Advanced tuning vars set: "
|
f"[bold yellow]⚠ Advanced tuning vars set: "
|
||||||
|
|||||||
+6
-4
@@ -57,9 +57,9 @@ def select_diverse_assets(
|
|||||||
Returns:
|
Returns:
|
||||||
List of selected assets
|
List of selected assets
|
||||||
"""
|
"""
|
||||||
# Fast path: fewer assets than limit
|
# Fast path: fewer assets than limit — sort for consistent ordering with other paths
|
||||||
if limit != "auto" and len(assets) <= limit:
|
if limit != "auto" and len(assets) <= limit:
|
||||||
return assets
|
return sorted(assets, key=lambda x: x.get("fileCreatedAt", ""))
|
||||||
|
|
||||||
# Sort by creation time
|
# Sort by creation time
|
||||||
assets = sorted(assets, key=lambda x: x.get("fileCreatedAt", ""))
|
assets = sorted(assets, key=lambda x: x.get("fileCreatedAt", ""))
|
||||||
@@ -483,8 +483,10 @@ def _cluster_aware_selection(
|
|||||||
norms = np.linalg.norm(emb_matrix, axis=1, keepdims=True)
|
norms = np.linalg.norm(emb_matrix, axis=1, keepdims=True)
|
||||||
emb_normed = emb_matrix / np.maximum(norms, 1e-8)
|
emb_normed = emb_matrix / np.maximum(norms, 1e-8)
|
||||||
|
|
||||||
# Build confidence weight array for hard example boosting
|
# Build confidence weight array for hard example boosting.
|
||||||
conf_array = np.ones(n)
|
# Default to 0.5 for faces with no confidence score so they receive a
|
||||||
|
# moderate diversity boost rather than being treated as high-confidence.
|
||||||
|
conf_array = np.full(n, 0.5)
|
||||||
if confidence_scores:
|
if confidence_scores:
|
||||||
for i, c in enumerate(confidence_scores):
|
for i, c in enumerate(confidence_scores):
|
||||||
if c is not None:
|
if c is not None:
|
||||||
|
|||||||
@@ -533,6 +533,8 @@ def upload_to_frigate(jobs: list[dict]) -> None:
|
|||||||
else:
|
else:
|
||||||
if pre_fscore is not None:
|
if pre_fscore is not None:
|
||||||
person_has_fscores = True
|
person_has_fscores = True
|
||||||
|
# Always record for reconcile so the Frigate filename→asset_id
|
||||||
|
# mapping is created even when the tracker write fails.
|
||||||
actually_uploaded.append((fname, asset_id))
|
actually_uploaded.append((fname, asset_id))
|
||||||
|
|
||||||
break
|
break
|
||||||
|
|||||||
+11
-3
@@ -57,8 +57,12 @@ def get_people() -> list[dict]:
|
|||||||
logger.error("Immich API key is invalid or expired (401 Unauthorized). Update API_KEY.")
|
logger.error("Immich API key is invalid or expired (401 Unauthorized). Update API_KEY.")
|
||||||
return []
|
return []
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
return resp.json().get("people", [])
|
data = resp.json()
|
||||||
except (requests.RequestException, ValueError) as e:
|
if not isinstance(data, dict):
|
||||||
|
logger.error("Unexpected response shape from Immich /people: %r", type(data))
|
||||||
|
return []
|
||||||
|
return data.get("people", [])
|
||||||
|
except (requests.RequestException, ValueError, AttributeError) as e:
|
||||||
logger.error("Failed to fetch people from Immich: %s", e)
|
logger.error("Failed to fetch people from Immich: %s", e)
|
||||||
return []
|
return []
|
||||||
|
|
||||||
@@ -118,7 +122,11 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
|
|||||||
logger.error("Error fetching assets for %s (page %s): %s", name, page, resp.status_code)
|
logger.error("Error fetching assets for %s (page %s): %s", name, page, resp.status_code)
|
||||||
break
|
break
|
||||||
|
|
||||||
page_assets = resp.json().get("assets", [])
|
body = resp.json()
|
||||||
|
if not isinstance(body, dict):
|
||||||
|
logger.error("Unexpected response shape fetching assets for %s (page %s): %r", name, page, type(body))
|
||||||
|
break
|
||||||
|
page_assets = body.get("assets", [])
|
||||||
# Immich ≥2.x returns {"assets": {"items": [...]}};
|
# Immich ≥2.x returns {"assets": {"items": [...]}};
|
||||||
# earlier versions returned {"assets": [...]} directly.
|
# earlier versions returned {"assets": [...]} directly.
|
||||||
if isinstance(page_assets, dict):
|
if isinstance(page_assets, dict):
|
||||||
|
|||||||
@@ -360,6 +360,8 @@ def find_by_crop_dimension(size: int) -> list[dict]:
|
|||||||
asset_to_frigate.setdefault(aid, fn) # first-seen wins; plain inversion silently drops duplicates
|
asset_to_frigate.setdefault(aid, fn) # first-seen wins; plain inversion silently drops duplicates
|
||||||
frigate_scores = entry.get("frigate_scores", {})
|
frigate_scores = entry.get("frigate_scores", {})
|
||||||
for asset_id, dims in entry.get("crop_dims", {}).items():
|
for asset_id, dims in entry.get("crop_dims", {}).items():
|
||||||
|
if not isinstance(dims, (list, tuple)) or len(dims) < 2:
|
||||||
|
continue
|
||||||
w, h = dims[0], dims[1]
|
w, h = dims[0], dims[1]
|
||||||
if w == size or h == size:
|
if w == size or h == size:
|
||||||
results.append({
|
results.append({
|
||||||
@@ -440,7 +442,7 @@ def reset_person(person_name: str) -> None:
|
|||||||
data["by_person"] = by_person
|
data["by_person"] = by_person
|
||||||
flat_key = _flat_key(filename)
|
flat_key = _flat_key(filename)
|
||||||
person_ids = set(_get_ids(tracker_entry))
|
person_ids = set(_get_ids(tracker_entry))
|
||||||
if person_ids and flat_key in data:
|
if person_ids and flat_key in data and isinstance(data[flat_key], list):
|
||||||
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)
|
||||||
changed = True
|
changed = True
|
||||||
|
|||||||
Reference in New Issue
Block a user