From a0083cb9fec3d92c4400d4298e5f36c8a73fecbc Mon Sep 17 00:00:00 2001 From: Holden Date: Sat, 13 Jun 2026 03:52:04 +0000 Subject: [PATCH] Fix three bugs found by code audit - Delete failure retry loop: when delete_frigate_person_files() fails, remove the file from the tracker so the next candidate targets a different worst file rather than re-attempting the same failed delete. - Interactive mode quality replacement: _configure_person() never set config["quality_replacement"], causing the executor to always default to False and silently skip all uploads for at-cap interactive jobs. Now mirrors auto_configure by reading Config.QUALITY_REPLACEMENT. - Silent mapping loss on API flap: after a successful upload, if the post-upload GET /api/faces returns None (transient API failure), the file was silently left unmapped. Now logs a warning so users know quality replacement won't target that file. Co-Authored-By: Claude Sonnet 4.6 --- winnow/executor.py | 28 +++++++++++++++++++--------- winnow/jobs.py | 2 +- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/winnow/executor.py b/winnow/executor.py index 3da9f80..5a3a751 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -293,6 +293,9 @@ def upload_to_frigate(jobs: list[dict]) -> None: known_frigate_files.discard(worst_frigate_file) else: logger.warning(f"Failed to delete {worst_frigate_file} for {name}, skipping replacement") + # Remove from tracker so the next candidate targets a different file. + # The file stays in Frigate (unmapped, like a manually-added file). + remove_frigate_file(name, worst_frigate_file) progress.advance(upload_task) continue @@ -315,16 +318,23 @@ def upload_to_frigate(jobs: list[dict]) -> None: # Identify the Frigate filename assigned to this upload # and record the mapping for future quality management. - current_files = set(get_frigate_person_files(name) or known_frigate_files) - new_files = current_files - known_frigate_files - if len(new_files) == 1 and asset_id: - record_frigate_file(name, next(iter(new_files)), asset_id) - elif len(new_files) > 1: - logger.info( - f"{name}: {len(new_files)} new Frigate files after uploading {fname}" - f" (concurrent upload detected) — skipping file mapping" + fresh = get_frigate_person_files(name) + if fresh is None: + logger.warning( + f"{name}: Frigate API unreachable after uploading {fname}" + f" — file mapping skipped, quality replacement won't target this file" ) - known_frigate_files = current_files + else: + current_files = set(fresh) + new_files = current_files - known_frigate_files + if len(new_files) == 1 and asset_id: + record_frigate_file(name, next(iter(new_files)), asset_id) + elif len(new_files) > 1: + logger.info( + f"{name}: {len(new_files)} new Frigate files after uploading {fname}" + f" (concurrent upload detected) — skipping file mapping" + ) + known_frigate_files = current_files break else: diff --git a/winnow/jobs.py b/winnow/jobs.py index 954e66e..369b566 100644 --- a/winnow/jobs.py +++ b/winnow/jobs.py @@ -139,7 +139,7 @@ def _configure_person(person: dict, people: list[dict]) -> dict | None: mode_choice = Prompt.ask("Choice", choices=["1", "2"], default="1") entity_type = "face" if mode_choice == "1" else "object" - config = {"name": name, "mode": entity_type} + config = {"name": name, "mode": entity_type, "quality_replacement": Config.QUALITY_REPLACEMENT} if entity_type == "object": config["object_class"] = Prompt.ask("Enter Object Class (e.g. dog, cat, car)", default="dog")