diff --git a/tests/test_upload_tracker.py b/tests/test_upload_tracker.py index b5b6acd..fcdd37c 100644 --- a/tests/test_upload_tracker.py +++ b/tests/test_upload_tracker.py @@ -156,3 +156,65 @@ def test_get_lowest_quality_mapped_file_skips_unscored(): result = get_lowest_quality_mapped_file("Alice") assert result is not None assert result[1] == "asset-scored" # only scored file is a candidate + + +# ── get_tracked_frigate_filenames ───────────────────────────────────────────── + +def test_get_tracked_frigate_filenames_empty(): + from winnow.upload_tracker import get_tracked_frigate_filenames + assert get_tracked_frigate_filenames("Alice") == set() + + +def test_get_tracked_frigate_filenames_returns_mapped(): + from winnow.upload_tracker import get_tracked_frigate_filenames, record_frigate_file + record_frigate_file("Alice", "Alice-1000.webp", "asset-a") + record_frigate_file("Alice", "Alice-1001.webp", "asset-b") + assert get_tracked_frigate_filenames("Alice") == {"Alice-1000.webp", "Alice-1001.webp"} + + +def test_get_tracked_frigate_filenames_excludes_removed(): + from winnow.upload_tracker import ( + get_tracked_frigate_filenames, + record_frigate_file, + remove_frigate_file, + ) + record_frigate_file("Alice", "Alice-1000.webp", "asset-a") + record_frigate_file("Alice", "Alice-1001.webp", "asset-b") + remove_frigate_file("Alice", "Alice-1000.webp") + assert get_tracked_frigate_filenames("Alice") == {"Alice-1001.webp"} + + +def test_get_tracked_frigate_filenames_isolated_by_person(): + from winnow.upload_tracker import get_tracked_frigate_filenames, record_frigate_file + record_frigate_file("Alice", "Alice-1000.webp", "asset-a") + record_frigate_file("Bob", "Bob-2000.webp", "asset-b") + assert get_tracked_frigate_filenames("Alice") == {"Alice-1000.webp"} + assert get_tracked_frigate_filenames("Bob") == {"Bob-2000.webp"} + + +# ── get_lowest_quality_mapped_file with exclude ─────────────────────────────── + +def test_get_lowest_quality_exclude_skips_specified_file(): + from winnow.upload_tracker import ( + get_lowest_quality_mapped_file, + mark_uploaded, + record_frigate_file, + ) + mark_uploaded("asset-lo", person_name="Alice", score=0.10) + mark_uploaded("asset-hi", person_name="Alice", score=0.90) + record_frigate_file("Alice", "Alice-lo.webp", "asset-lo") + record_frigate_file("Alice", "Alice-hi.webp", "asset-hi") + result = get_lowest_quality_mapped_file("Alice", exclude={"Alice-lo.webp"}) + assert result is not None + assert result[1] == "asset-hi" # lo was excluded; hi is returned + + +def test_get_lowest_quality_exclude_all_returns_none(): + from winnow.upload_tracker import ( + get_lowest_quality_mapped_file, + mark_uploaded, + record_frigate_file, + ) + mark_uploaded("asset-a", person_name="Alice", score=0.50) + record_frigate_file("Alice", "Alice-a.webp", "asset-a") + assert get_lowest_quality_mapped_file("Alice", exclude={"Alice-a.webp"}) is None diff --git a/winnow/executor.py b/winnow/executor.py index fbf6e40..0289061 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -17,9 +17,11 @@ from .frigate_api import delete_frigate_person_files, get_frigate_person_files from .image_processing import process_face_mode, process_full_mode, process_object_mode from .immich_api import fetch_face_data, fetch_full_image from .log_config import console +from .quality import assess_quality from .upload_tracker import ( get_lowest_quality_mapped_file, get_tracked_frigate_file_count, + get_tracked_frigate_filenames, mark_rejected, mark_uploaded, record_frigate_file, @@ -205,7 +207,22 @@ def execute_jobs(jobs: list[dict]) -> None: # Record which asset produced which output file filename = f"{count}.jpg" asset_map[filename] = asset["id"] - score_map[filename] = asset.get("quality_score") or asset.get("face_confidence") + score_map[filename] = asset.get("quality_score") + # Time-spread path: compute blur score from the downloaded + # image. Cap at 1440px so the scale matches the preview + # thumbnails the embedding path uses for scoring — Laplacian + # variance grows with resolution, making full-res and + # thumbnail scores incomparable if left uncapped. + if mode == "face" and score_map[filename] is None: + try: + score_img = img.convert("RGB") if img.mode != "RGB" else img + if score_img.width > 1440 or score_img.height > 1440: + score_img = score_img.copy() + score_img.thumbnail((1440, 1440), Image.LANCZOS) + score_map[filename] = assess_quality(score_img).blur_score + except Exception as exc: + logger.debug(f"Quality score fallback for {asset['id']}: {exc}") + score_map[filename] = 0.0 # unknown quality — treat as lowest # Also record object-mode variant filenames if mode == "object": for f in sorted(os.listdir(person_dir)): @@ -320,14 +337,41 @@ def upload_to_frigate(jobs: list[dict]) -> None: # Snapshot live Frigate files for post-upload reconciliation diff only. # effective_count is sourced from the tracker (mapped files) so that # manually-added Frigate files don't consume winnow's managed quota. - known_frigate_files_at_start: set[str] = set(get_frigate_person_files(name) or []) + _snapshot = get_frigate_person_files(name) + if _snapshot is None: + # Frigate GET is down; fall back to the tracker's mapped filenames + # as the pre-upload baseline. reconciliation will still work unless + # there are concurrent manual uploads (handled by >target guard). + logger.warning( + f"{name}: Frigate API unreachable at upload start" + " — using tracker baseline for post-upload reconciliation" + ) + known_frigate_files_at_start: set[str] = get_tracked_frigate_filenames(name) + else: + known_frigate_files_at_start: set[str] = set(_snapshot) effective_count = get_tracked_frigate_file_count(name) quality_replacement = job.get("config", {}).get("quality_replacement", False) actually_uploaded: list[tuple[str, str | None]] = [] + failed_deletes: set[str] = set() + min_quality_score_for_slot: float | None = None for fname in person_files: fpath = os.path.join(person_dir, fname) + # If a previous replacement delete succeeded but that upload failed, + # require the next candidate to beat the deleted file's score so the + # freed slot isn't filled with something worse than what we removed. + if min_quality_score_for_slot is not None: + file_score = score_map.get(fname) + if file_score is None or file_score <= min_quality_score_for_slot: + score_str = f"{file_score:.3f}" if file_score is not None else "N/A" + progress.console.print( + f" [dim]⏭ {fname}: score {score_str} ≤ freed slot floor" + f" {min_quality_score_for_slot:.3f}, skipping[/dim]" + ) + progress.advance(upload_task) + continue + at_cap = effective_count >= Config.MAX_AUTO_IMAGES if at_cap: if not quality_replacement: @@ -339,7 +383,7 @@ def upload_to_frigate(jobs: list[dict]) -> None: progress.console.print(f" [dim]⏭ {fname}: no confidence score, skipping replacement[/dim]") progress.advance(upload_task) continue - worst = get_lowest_quality_mapped_file(name) + worst = get_lowest_quality_mapped_file(name, exclude=failed_deletes) if worst is None or new_score <= worst[2]: worst_score_str = f"{worst[2]:.3f}" if worst is not None else "N/A" progress.console.print( @@ -357,11 +401,10 @@ def upload_to_frigate(jobs: list[dict]) -> None: if delete_frigate_person_files(name, [worst_frigate_file]): remove_frigate_file(name, worst_frigate_file) effective_count -= 1 + min_quality_score_for_slot = worst_score 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) + failed_deletes.add(worst_frigate_file) progress.advance(upload_task) continue @@ -377,6 +420,7 @@ def upload_to_frigate(jobs: list[dict]) -> None: uploaded += 1 person_uploaded += 1 effective_count += 1 + min_quality_score_for_slot = None asset_id = asset_map.get(fname) if asset_id: @@ -438,7 +482,13 @@ def upload_to_frigate(jobs: list[dict]) -> None: progress.advance(upload_task) - # Batch-map Frigate filenames to asset IDs now that all uploads are done + if min_quality_score_for_slot is not None: + logger.warning( + f"{name}: freed replacement slot (floor {min_quality_score_for_slot:.3f})" + " was not filled this run — will be available next run" + ) + + # Batch-map Frigate filenames to asset IDs now that all uploads are done. if actually_uploaded: _reconcile_frigate_mappings(name, known_frigate_files_at_start, actually_uploaded) diff --git a/winnow/frigate_api.py b/winnow/frigate_api.py index c51115e..b6d4950 100644 --- a/winnow/frigate_api.py +++ b/winnow/frigate_api.py @@ -73,6 +73,11 @@ def delete_frigate_person_files(person_name: str, filenames: list[str]) -> bool: if resp.ok: logger.debug(f"Deleted {len(filenames)} Frigate file(s) for {person_name}") return True + if resp.status_code == 404: + # File already absent — stale tracker entry. Return True so the caller + # removes it from the tracker and frees the slot cleanly. + logger.warning(f"Frigate file(s) not found for {person_name} (stale tracker entry?): {filenames}") + return True logger.warning(f"Frigate delete returned {resp.status_code} for {person_name}") return False except Exception as e: diff --git a/winnow/upload_tracker.py b/winnow/upload_tracker.py index 3b6de12..aa450e9 100644 --- a/winnow/upload_tracker.py +++ b/winnow/upload_tracker.py @@ -12,7 +12,7 @@ Both are excluded from future candidate pools. To reset: by_person schema (frigate_uploaded_ids.json): { "asset_ids": ["immich-id-1", ...], # all assets we attempted to upload - "scores": {"immich-id-1": 0.953}, # Immich face confidence at upload time + "scores": {"immich-id-1": 450.3}, # Laplacian blur variance at upload time "frigate_files": {"PersonName-123.webp": "immich-id-1"}, # Frigate filename → asset ID "frigate_count": 42 # last known Frigate training image count } @@ -158,9 +158,26 @@ def get_tracked_frigate_file_count(person_name: str) -> int: return len(entry["frigate_files"]) -def get_lowest_quality_mapped_file(person_name: str) -> tuple[str, str, float] | None: +def get_tracked_frigate_filenames(person_name: str) -> set[str]: + """Return the set of Frigate filenames currently mapped in the tracker for a person. + + Used as a pre-upload baseline when the Frigate GET API is unreachable at + upload start, so reconciliation can still identify newly uploaded files. + """ + data = _load(UPLOAD_TRACKER_FILE) + entry = _migrate_entry(data.get("by_person", {}).get(person_name, {})) + return set(entry["frigate_files"].keys()) + + +def get_lowest_quality_mapped_file( + person_name: str, exclude: set[str] | None = None +) -> tuple[str, str, float] | None: """Return (frigate_filename, asset_id, score) for the mapped file with the lowest - confidence score, or None if no mapped files with known scores exist.""" + quality score, or None if no mapped files with known scores exist. + + Pass `exclude` to skip files that failed to delete this run without removing + them from the tracker — they remain candidates on the next run. + """ data = _load(UPLOAD_TRACKER_FILE) entry = _migrate_entry(data.get("by_person", {}).get(person_name, {})) frigate_files = entry.get("frigate_files", {}) @@ -168,7 +185,7 @@ def get_lowest_quality_mapped_file(person_name: str) -> tuple[str, str, float] | candidates = [ (frigate_filename, asset_id, scores[asset_id]) for frigate_filename, asset_id in frigate_files.items() - if asset_id in scores + if asset_id in scores and (exclude is None or frigate_filename not in exclude) ] if not candidates: return None