Compare commits

..
10 Commits
Author SHA1 Message Date
flan 3c0ef47fdc release: v0.6.3 2026-06-16 18:13:42 +00:00
flan cf7660595d chore: update lockfile 2026-06-16 18:13:42 +00:00
flan 14f759e960 fix: address 3 quality review findings — record_frigate_files_batch cache mutation, tracker_ok flag, LIMIT guard
- record_frigate_files_batch: copy-before-mutate so a write failure
  doesn't leave cache ahead of disk (same fix as remove_frigate_files_batch)
- executor: replace tracker_ok boolean with try/else
- jobs: collapse duplicate custom_limit is not None checks into one guard

Bump version to 0.6.3.
2026-06-16 18:13:36 +00:00
flan e8cb390fe4 fix: address 2 quality review findings — begin_batch dirty guard, LIMIT<=0 warning 2026-06-16 18:04:38 +00:00
flan 54b52b0a73 fix: address 3 quality review findings — batch reject tracker, skip flush when clean, hoist frigate url check 2026-06-16 17:55:34 +00:00
flan b622e58f1b fix: address 3 quality review findings — flush_batch finally guard, _laplacian_var helper, has_frigate_scores no-copy 2026-06-16 17:09:00 +00:00
flan f3622b8d41 fix: address 4 quality review findings — flush_batch order, batch finally guard, cache copy, LIMIT=0 fallthrough 2026-06-16 16:59:43 +00:00
flan 817fa17e41 fix: address 3 quality review findings — tracker_ok gate, LIMIT=0 warning, cache write log level 2026-06-16 16:41:32 +00:00
flan 8846a4f1df fix: address 3 post-fix audit findings — begin_batch flush guard, misleading debug log, shared asset_id score deletion 2026-06-16 16:19:54 +00:00
flan a6bae5da05 fix: address 10 audit findings — import bug, fscore stale flag, cache mutation, batch safety, falsy guards 2026-06-16 16:16:28 +00:00
9 changed files with 311 additions and 255 deletions
+10
View File
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] ## [Unreleased]
## [0.6.3] - 2026-06-16
### Fixed
- **`record_frigate_files_batch` no longer mutates the tracker cache before write**: the function shared the same cache-corruption-on-write-failure bug that was fixed in `remove_frigate_files_batch` in v0.6.1 — `data.setdefault("by_person", {})` mutated the cached dict in-place, so a disk-full or permission error left the in-memory cache ahead of the on-disk file. Now uses the same copy-before-mutate pattern (shallow copies of the top-level dict and `by_person` sub-dict) so a failed write leaves cache and disk in sync.
- **`tracker_ok` boolean flag replaced with try/else**: the intermediate boolean was a misleading placeholder — the `True` initial value suggested success before the operation ran. The control flow is now expressed directly with a try/except/else block.
- **`LIMIT` env var guard simplified**: the two adjacent `if custom_limit is not None` checks in `_resolve_strategy` are collapsed into a single `if custom_limit is not None:` with nested branches, removing redundant evaluation.
## [0.6.2] - 2026-06-16 ## [0.6.2] - 2026-06-16
### Changed ### Changed
+1 -1
View File
@@ -1,6 +1,6 @@
[project] [project]
name = "winnow" name = "winnow"
version = "0.6.2" version = "0.6.3"
description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition." description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition."
license = "AGPL-3.0-or-later" license = "AGPL-3.0-or-later"
requires-python = ">=3.13" requires-python = ">=3.13"
Generated
+1 -1
View File
@@ -862,7 +862,7 @@ wheels = [
[[package]] [[package]]
name = "winnow" name = "winnow"
version = "0.6.1" version = "0.6.2"
source = { editable = "." } source = { editable = "." }
dependencies = [ dependencies = [
{ name = "croniter" }, { name = "croniter" },
+1 -1
View File
@@ -90,7 +90,7 @@ class EmbeddingCache:
np.save(tmp, embedding) np.save(tmp, embedding)
os.replace(tmp, final) os.replace(tmp, final)
except Exception as e: except Exception as e:
logger.debug("Cache write failed for %s: %s", asset_id, e) logger.warning("Cache write failed for %s: %s", asset_id, e)
try: try:
os.remove(tmp) os.remove(tmp)
except OSError: except OSError:
+4 -3
View File
@@ -86,6 +86,8 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
for p in sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)[1:] for p in sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)[1:]
} }
skip_ids = _smaller_duplicate_ids(duplicates)
if not Config.MERGE_DUPLICATE_PEOPLE: if not Config.MERGE_DUPLICATE_PEOPLE:
rprint("\n[bold yellow]⚠ Duplicate person names detected in Immich:[/bold yellow]") rprint("\n[bold yellow]⚠ Duplicate person names detected in Immich:[/bold yellow]")
for name, ps in sorted(duplicates.items()): for name, ps in sorted(duplicates.items()):
@@ -107,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 _smaller_duplicate_ids(duplicates)] return [p for p in people if p["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
@@ -133,7 +135,6 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
# 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
# this filter is a no-op for them. # this filter is a no-op for them.
skip_ids = _smaller_duplicate_ids(duplicates)
return [p for p in fresh if p.get("id") not in skip_ids] return [p for p in fresh if p.get("id") not in skip_ids]
# All merges failed — fall back to local deduplication (keep largest per name) so # All merges failed — fall back to local deduplication (keep largest per name) so
@@ -142,7 +143,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
" [yellow]All merges failed — applying local deduplication" " [yellow]All merges failed — applying local deduplication"
" to avoid overwriting output.[/yellow]" " to avoid overwriting output.[/yellow]"
) )
return [p for p in people if p["id"] not in _smaller_duplicate_ids(duplicates)] return [p for p in people if p["id"] not in skip_ids]
_UNSUPPORTED_VARS = [ _UNSUPPORTED_VARS = [
+217 -203
View File
@@ -27,6 +27,8 @@ from .log_config import console
from .quality import blur_score_from_image from .quality import blur_score_from_image
from .reconcile import enrich_asset_with_face_data, reconcile_frigate_mappings from .reconcile import enrich_asset_with_face_data, reconcile_frigate_mappings
from .upload_tracker import ( from .upload_tracker import (
UPLOAD_TRACKER_FILE,
REJECT_TRACKER_FILE,
get_lowest_quality_mapped_file, get_lowest_quality_mapped_file,
get_most_redundant_mapped_file, get_most_redundant_mapped_file,
get_tracked_frigate_file_count, get_tracked_frigate_file_count,
@@ -367,233 +369,245 @@ def upload_to_frigate(jobs: list[dict]) -> None:
person_has_fscores: bool = has_frigate_scores(name) person_has_fscores: bool = has_frigate_scores(name)
begin_batch(UPLOAD_TRACKER_FILE) begin_batch(UPLOAD_TRACKER_FILE)
for fname in person_files: begin_batch(REJECT_TRACKER_FILE)
fpath = os.path.join(person_dir, fname) try:
for fname in person_files:
fpath = os.path.join(person_dir, fname)
# If a previous replacement delete succeeded but that upload failed, # If a previous replacement delete succeeded but that upload failed,
# require the next candidate to beat the deleted file's score so the # 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. # freed slot isn't filled with something worse than what we removed.
if min_quality_score_for_slot is not None: if min_quality_score_for_slot is not None:
file_score = score_map.get(fname) file_score = score_map.get(fname)
if file_score is not None and file_score <= min_quality_score_for_slot: if file_score is not None and file_score <= min_quality_score_for_slot:
progress.console.print( progress.console.print(
f" [dim]⏭ {fname}: score {file_score:.3f} ≤ freed slot floor" f" [dim]⏭ {fname}: score {file_score:.3f} ≤ freed slot floor"
f" {min_quality_score_for_slot:.3f}, skipping[/dim]" f" {min_quality_score_for_slot:.3f}, skipping[/dim]"
) )
progress.advance(upload_task) progress.advance(upload_task)
continue continue
at_cap = effective_count >= Config.MAX_AUTO_IMAGES at_cap = effective_count >= Config.MAX_AUTO_IMAGES
# Pre-upload Frigate score — clean measurement (image not yet in training set). # Pre-upload Frigate score — clean measurement (image not yet in training set).
# Called for all below-cap uploads (seeds frigate_scores for future at-cap # Called for all below-cap uploads (seeds frigate_scores for future at-cap
# replacement) and for at-cap uploads when scores already exist. Skipped on # replacement) and for at-cap uploads when scores already exist. Skipped on
# the first run (pre_run_count == 0) since Frigate has no model yet. # the first run (pre_run_count == 0) since Frigate has no model yet.
# recognize_face returns (face_name, score); we only use the score when the # recognize_face returns (face_name, score); we only use the score when the
# best match is for the correct person. Mismatches (or "unknown") are treated # best match is for the correct person. Mismatches (or "unknown") are treated
# as None so a wrong-person score never drives a ceiling skip or replacement. # as None so a wrong-person score never drives a ceiling skip or replacement.
# Frigate rebuilds its model asynchronously after any delete (clear + background # Frigate rebuilds its model asynchronously after any delete (clear + background
# thread), so the first recognize call after a deletion returns None — our code # thread), so the first recognize call after a deletion returns None — our code
# handles this conservatively by skipping that candidate until the next run. # handles this conservatively by skipping that candidate until the next run.
# LIMITATION — async rebuild during multi-replacement runs: each deletion in a # LIMITATION — async rebuild during multi-replacement runs: each deletion in a
# single run triggers a background model rebuild in Frigate. Subsequent recognize # single run triggers a background model rebuild in Frigate. Subsequent recognize
# calls in the same run may get None (rebuild in progress), causing later # calls in the same run may get None (rebuild in progress), causing later
# candidates to fall back to blur-score replacement or be skipped entirely. # candidates to fall back to blur-score replacement or be skipped entirely.
# The more replacements that happen in one run, the worse the scoring gets. # The more replacements that happen in one run, the worse the scoring gets.
# TODO(frigate-api): if Frigate exposes a model generation counter or a # TODO(frigate-api): if Frigate exposes a model generation counter or a
# rebuild-complete signal, poll it between recognize calls during replacement # rebuild-complete signal, poll it between recognize calls during replacement
# sequences rather than accepting stale/None scores. # sequences rather than accepting stale/None scores.
pre_fscore: float | None = None pre_fscore: float | None = None
if Config.ENABLE_FRIGATE_SCORES and pre_run_count > 0: if Config.ENABLE_FRIGATE_SCORES and pre_run_count > 0:
if not at_cap or person_has_fscores: if not at_cap or person_has_fscores:
_result = recognize_face(fpath) _result = recognize_face(fpath)
if _result is not None and (_result[0] or "").casefold() == name.casefold(): if _result is not None and (_result[0] or "").casefold() == name.casefold():
pre_fscore = _result[1] pre_fscore = _result[1]
# Below-cap novelty gate: skip candidates already covered by the Frigate model, # Below-cap novelty gate: skip candidates already covered by the Frigate model,
# including conditions learned from manually-added images winnow can't track. # including conditions learned from manually-added images winnow can't track.
# pre_fscore is None on the first run (pre_run_count == 0 skips recognize_face # pre_fscore is None on the first run (pre_run_count == 0 skips recognize_face
# above), so this block never fires on the first run without an extra guard. # above), so this block never fires on the first run without an extra guard.
if not at_cap and pre_fscore is not None: if not at_cap and pre_fscore is not None:
_ceiling = Config.FRIGATE_SCORE_CEILING _ceiling = Config.FRIGATE_SCORE_CEILING
if _ceiling is None: if _ceiling is None:
# Dynamic default: bar = most-redundant tracked file's Frigate score. # Dynamic default: bar = most-redundant tracked file's Frigate score.
# Falls back to uploading freely when no tracked scores exist yet. # Falls back to uploading freely when no tracked scores exist yet.
_bar = get_most_redundant_mapped_file(name) _bar = get_most_redundant_mapped_file(name)
_skip = _bar is not None and pre_fscore > _bar[2] _skip = _bar is not None and pre_fscore > _bar[2]
_bar_str = f"most redundant tracked {_bar[2]:.2f}" if _bar else "" _bar_str = f"most redundant tracked {_bar[2]:.2f}" if _bar else ""
elif _ceiling == 0.0: elif _ceiling == 0.0:
_skip = False # explicitly disabled _skip = False # explicitly disabled
_bar_str = "" _bar_str = ""
else: else:
_skip = pre_fscore > _ceiling _skip = pre_fscore > _ceiling
_bar_str = f"ceiling {_ceiling:.2f}" _bar_str = f"ceiling {_ceiling:.2f}"
if _skip: if _skip:
progress.console.print( progress.console.print(
f" [dim]⏭ {fname}: Frigate score {pre_fscore:.2f}" f" [dim]⏭ {fname}: Frigate score {pre_fscore:.2f}"
f" > {_bar_str}, already covered[/dim]" f" > {_bar_str}, already covered[/dim]"
) )
progress.advance(upload_task) progress.advance(upload_task)
continue continue
if at_cap: if at_cap:
if not quality_replacement: if not quality_replacement:
progress.console.print(f" [dim]⏭ {fname}: at cap, quality replacement disabled[/dim]") progress.console.print(f" [dim]⏭ {fname}: at cap, quality replacement disabled[/dim]")
progress.advance(upload_task) progress.advance(upload_task)
continue continue
using_fscore = person_has_fscores and Config.ENABLE_FRIGATE_SCORES using_fscore = person_has_fscores and Config.ENABLE_FRIGATE_SCORES
if using_fscore: if using_fscore:
candidate_score = pre_fscore candidate_score = pre_fscore
get_target = get_most_redundant_mapped_file get_target = get_most_redundant_mapped_file
score_label, better_note = "frigate", " (more novel)" score_label, better_note = "frigate", " (more novel)"
no_score_msg = "Frigate recognize unavailable, skipping replacement" no_score_msg = "Frigate recognize unavailable, skipping replacement"
is_better_than = lambda c, t: c < t is_better_than = lambda c, t: c < t
else: else:
candidate_score = score_map.get(fname) candidate_score = score_map.get(fname)
get_target = get_lowest_quality_mapped_file get_target = get_lowest_quality_mapped_file
score_label, better_note = "blur", "" score_label, better_note = "blur", ""
no_score_msg = "no quality score, skipping replacement" no_score_msg = "no quality score, skipping replacement"
is_better_than = lambda c, t: c > t is_better_than = lambda c, t: c > t
if candidate_score is None: if candidate_score is None:
progress.console.print(f" [dim]⏭ {fname}: {no_score_msg}[/dim]") progress.console.print(f" [dim]⏭ {fname}: {no_score_msg}[/dim]")
progress.advance(upload_task) progress.advance(upload_task)
continue continue
target = get_target(name, exclude=failed_deletes) target = get_target(name, exclude=failed_deletes)
not_better = target is None or not is_better_than(candidate_score, target[2]) not_better = target is None or not is_better_than(candidate_score, target[2])
if not_better: if not_better:
target_str = f"{target[2]:.3f}" if target is not None else "N/A" target_str = f"{target[2]:.3f}" if target is not None else "N/A"
cmp_op = "<" if using_fscore else ">"
progress.console.print(
f" [dim]⏭ {fname}: {score_label} {candidate_score:.3f}"
f" not {cmp_op} {target_str}, skipping[/dim]"
)
progress.advance(upload_task)
continue
target_frigate_file, _target_asset_id, target_score = target
cmp_op = "<" if using_fscore else ">" cmp_op = "<" if using_fscore else ">"
progress.console.print( progress.console.print(
f" [dim]⏭ {fname}: {score_label} {candidate_score:.3f}" f" 🔄 {fname}: {score_label} {candidate_score:.3f} {cmp_op} {target_score:.3f},"
f" not {cmp_op} {target_str}, skipping[/dim]" f" replacing {target_frigate_file}{better_note}"
) )
progress.advance(upload_task) if delete_frigate_person_files(name, [target_frigate_file]):
continue remove_frigate_file(name, target_frigate_file)
person_has_fscores = has_frigate_scores(name)
target_frigate_file, _target_asset_id, target_score = target effective_count -= 1
cmp_op = "<" if using_fscore else ">" min_quality_score_for_slot = None if using_fscore else target_score
progress.console.print(
f" 🔄 {fname}: {score_label} {candidate_score:.3f} {cmp_op} {target_score:.3f},"
f" replacing {target_frigate_file}{better_note}"
)
if delete_frigate_person_files(name, [target_frigate_file]):
remove_frigate_file(name, target_frigate_file)
effective_count -= 1
min_quality_score_for_slot = None if using_fscore else target_score
else:
logger.warning("Failed to delete %s for %s, skipping replacement", target_frigate_file, name)
failed_deletes.add(target_frigate_file)
progress.advance(upload_task)
continue
for attempt in range(1, max_retries + 1):
try:
with open(fpath, "rb") as f:
resp = requests.post(
f"{frigate_url}/api/faces/{encoded_name}/register",
files={"file": (fname, f, "image/jpeg")},
timeout=30,
)
if resp.status_code == 200:
uploaded += 1
person_uploaded += 1
effective_count += 1
min_quality_score_for_slot = None
asset_id = asset_map.get(fname)
if asset_id:
try:
mark_uploaded(
asset_id,
person_name=name,
score=score_map.get(fname),
crop_dims=dims_map.get(fname),
frigate_score=pre_fscore,
)
except Exception as tracker_exc:
# Upload to Frigate succeeded — don't retry on tracker
# failure or we'd upload a duplicate to Frigate.
logger.error(
"Tracker write failed for %s — upload succeeded"
" but asset may be re-selected next run: %s",
fname, tracker_exc,
)
if pre_fscore is not None:
person_has_fscores = True
actually_uploaded.append((fname, asset_id))
break
else: else:
logger.warning("Failed to delete %s for %s, skipping replacement", target_frigate_file, name)
failed_deletes.add(target_frigate_file)
progress.advance(upload_task)
continue
for attempt in range(1, max_retries + 1):
try:
with open(fpath, "rb") as f:
resp = requests.post(
f"{frigate_url}/api/faces/{encoded_name}/register",
files={"file": (fname, f, "image/jpeg")},
timeout=30,
)
if resp.status_code == 200:
uploaded += 1
person_uploaded += 1
effective_count += 1
min_quality_score_for_slot = None
asset_id = asset_map.get(fname)
if asset_id:
try:
mark_uploaded(
asset_id,
person_name=name,
score=score_map.get(fname),
crop_dims=dims_map.get(fname),
frigate_score=pre_fscore,
)
except Exception as tracker_exc:
# Upload to Frigate succeeded — don't retry on tracker
# failure or we'd upload a duplicate to Frigate.
logger.error(
"Tracker write failed for %s — upload succeeded"
" but asset may be re-selected next run: %s",
fname, tracker_exc,
)
else:
if pre_fscore is not None:
person_has_fscores = True
actually_uploaded.append((fname, asset_id))
break
else:
if attempt < max_retries:
logger.warning(
f"Upload attempt {attempt}/{max_retries} for {fname}:"
f" HTTP {resp.status_code}, retrying..."
)
continue
failed += 1
person_failed += 1
progress.console.print(
f" [red]✗ {fname}: HTTP {resp.status_code} (after {max_retries} attempts)[/red]"
)
full_body = resp.text
try:
error_detail = resp.json().get("message", full_body[:100])
except Exception:
error_detail = full_body[:100]
if resp.status_code == 400:
progress.console.print(f" [dim]{error_detail}[/dim]")
else:
logger.debug("%s HTTP %s: %s", fname, resp.status_code, error_detail)
_is_permanent = (
(resp.status_code == 400 and "face" in full_body.lower())
or resp.status_code == 422
)
if _is_permanent:
asset_id = asset_map.get(fname)
if asset_id:
mark_rejected(asset_id, person_name=name)
except (requests.exceptions.ConnectionError, requests.exceptions.Timeout) as exc:
if attempt < max_retries: if attempt < max_retries:
logger.warning( logger.warning(
f"Upload attempt {attempt}/{max_retries} for {fname}:" f"Upload attempt {attempt}/{max_retries} for {fname}:"
f" HTTP {resp.status_code}, retrying..." f" {type(exc).__name__}, retrying..."
)
continue
failed += 1
person_failed += 1
label = (
"Connection refused"
if isinstance(exc, requests.exceptions.ConnectionError)
else "Request timed out (30s)"
)
progress.console.print(
f" [red]✗ {fname}: {label} (after {max_retries} attempts)[/red]"
)
except Exception as e:
if attempt < max_retries:
logger.warning(
f"Upload attempt {attempt}/{max_retries} for {fname}:"
f" {type(e).__name__}, retrying..."
) )
continue continue
failed += 1 failed += 1
person_failed += 1 person_failed += 1
progress.console.print( progress.console.print(
f" [red]✗ {fname}: HTTP {resp.status_code} (after {max_retries} attempts)[/red]" f" [red]✗ {fname}: {type(e).__name__} - {e} (after {max_retries} attempts)[/red]"
) )
full_body = resp.text
try:
error_detail = resp.json().get("message", full_body[:100])
except Exception:
error_detail = full_body[:100]
if resp.status_code == 400:
progress.console.print(f" [dim]{error_detail}[/dim]")
else:
logger.debug("%s HTTP %s: %s", fname, resp.status_code, error_detail)
_is_permanent = (
(resp.status_code == 400 and "face" in full_body.lower())
or resp.status_code == 422
)
if _is_permanent:
asset_id = asset_map.get(fname)
if asset_id:
mark_rejected(asset_id, person_name=name)
except (requests.exceptions.ConnectionError, requests.exceptions.Timeout) as exc:
if attempt < max_retries:
logger.warning(
f"Upload attempt {attempt}/{max_retries} for {fname}:"
f" {type(exc).__name__}, retrying..."
)
continue
failed += 1
person_failed += 1
label = (
"Connection refused"
if isinstance(exc, requests.exceptions.ConnectionError)
else "Request timed out (30s)"
)
progress.console.print(
f" [red]✗ {fname}: {label} (after {max_retries} attempts)[/red]"
)
except Exception as e:
if attempt < max_retries:
logger.warning(
f"Upload attempt {attempt}/{max_retries} for {fname}:"
f" {type(e).__name__}, retrying..."
)
continue
failed += 1
person_failed += 1
progress.console.print(
f" [red]✗ {fname}: {type(e).__name__} - {e} (after {max_retries} attempts)[/red]"
)
progress.advance(upload_task) progress.advance(upload_task)
if min_quality_score_for_slot is not None: if min_quality_score_for_slot is not None:
logger.warning( logger.warning(
f"{name}: freed replacement slot (floor {min_quality_score_for_slot:.3f})" f"{name}: freed replacement slot (floor {min_quality_score_for_slot:.3f})"
" was not filled this run — will be available next run" " was not filled this run — will be available next run"
) )
flush_batch(UPLOAD_TRACKER_FILE) finally:
try:
flush_batch(UPLOAD_TRACKER_FILE)
except Exception as _flush_exc:
logger.warning("flush_batch failed during cleanup — batch will be recovered on next begin_batch: %s", _flush_exc)
try:
flush_batch(REJECT_TRACKER_FILE)
except Exception as _flush_exc:
logger.warning("flush_batch failed during cleanup — batch will be recovered on next begin_batch: %s", _flush_exc)
# Batch-map Frigate filenames to asset IDs now that all uploads are done. # Batch-map Frigate filenames to asset IDs now that all uploads are done.
if actually_uploaded and not _skip_reconcile: if actually_uploaded and not _skip_reconcile:
+3 -1
View File
@@ -70,7 +70,9 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st
custom_limit = _getenv_optional_int("LIMIT") custom_limit = _getenv_optional_int("LIMIT")
if custom_limit is not None: if custom_limit is not None:
return custom_limit, "smart" if custom_limit > 0:
return custom_limit, "smart"
logger.warning("LIMIT=%s is invalid — ignoring and using auto strategy", custom_limit)
strategy_map = { strategy_map = {
"adaptive": ("auto", "smart"), "adaptive": ("auto", "smart"),
+8 -6
View File
@@ -14,6 +14,11 @@ from PIL import Image
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
def _laplacian_var(img_np: np.ndarray) -> float:
gray = cv2.cvtColor(img_np, cv2.COLOR_RGB2GRAY) if img_np.ndim == 3 else img_np
return float(cv2.Laplacian(gray, cv2.CV_64F).var())
@dataclass @dataclass
class QualityResult: class QualityResult:
"""Result of quality assessment on a face/image crop.""" """Result of quality assessment on a face/image crop."""
@@ -32,8 +37,7 @@ def check_blur(img_np: np.ndarray, threshold: float = 100.0) -> tuple[bool, str]
Lower variance = blurrier image. ArcFace needs clear facial features. Lower variance = blurrier image. ArcFace needs clear facial features.
""" """
gray = cv2.cvtColor(img_np, cv2.COLOR_RGB2GRAY) if img_np.ndim == 3 else img_np variance = _laplacian_var(img_np)
variance = cv2.Laplacian(gray, cv2.CV_64F).var()
if variance < threshold: if variance < threshold:
return False, f"Blurry (laplacian={variance:.1f}, threshold={threshold})" return False, f"Blurry (laplacian={variance:.1f}, threshold={threshold})"
return True, "" return True, ""
@@ -115,8 +119,7 @@ def assess_quality(
reasons = [] reasons = []
# Compute laplacian variance once (used by check_blur and stored as blur_score) # Compute laplacian variance once (used by check_blur and stored as blur_score)
gray = cv2.cvtColor(img_np, cv2.COLOR_RGB2GRAY) if img_np.ndim == 3 else img_np blur_score = _laplacian_var(img_np)
blur_score = float(cv2.Laplacian(gray, cv2.CV_64F).var())
checks = [ checks = [
( (
@@ -154,8 +157,7 @@ def blur_score_from_image(img: Image.Image, max_dim: int = 1440) -> float | None
if score_img.width > max_dim or score_img.height > max_dim: if score_img.width > max_dim or score_img.height > max_dim:
score_img = score_img.copy() score_img = score_img.copy()
score_img.thumbnail((max_dim, max_dim), Image.LANCZOS) score_img.thumbnail((max_dim, max_dim), Image.LANCZOS)
gray = cv2.cvtColor(np.array(score_img), cv2.COLOR_RGB2GRAY) return _laplacian_var(np.array(score_img))
return float(cv2.Laplacian(gray, cv2.CV_64F).var())
except Exception as exc: except Exception as exc:
logger.debug("blur_score_from_image failed: %s", exc) logger.debug("blur_score_from_image failed: %s", exc)
return None return None
+66 -39
View File
@@ -32,7 +32,7 @@ import logging
import os import os
from pathlib import Path from pathlib import Path
from .frigate_api import delete_frigate_person_files from .frigate_api import _get_frigate_url, delete_frigate_person_files
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -44,6 +44,7 @@ REJECT_TRACKER_FILE = "frigate_rejected_ids.json"
# Keyed by full path so tests with isolated tmp dirs never share entries. # Keyed by full path so tests with isolated tmp dirs never share entries.
_cache: dict[str, dict] = {} _cache: dict[str, dict] = {}
_deferred: set[str] = set() # paths whose disk writes are batched until flush_batch() _deferred: set[str] = set() # paths whose disk writes are batched until flush_batch()
_dirty: set[str] = set() # deferred paths that received at least one _save during the batch
def _tracker_path(filename: str) -> Path: def _tracker_path(filename: str) -> Path:
@@ -87,6 +88,7 @@ def _save(filename: str, data: dict) -> None:
key = str(path) key = str(path)
if key in _deferred: if key in _deferred:
_cache[key] = data # accumulate in cache; disk write deferred until flush_batch() _cache[key] = data # accumulate in cache; disk write deferred until flush_batch()
_dirty.add(key)
return return
_write_to_disk(path, data) _write_to_disk(path, data)
_cache[key] = data # update cache only after successful write _cache[key] = data # update cache only after successful write
@@ -95,17 +97,32 @@ def _save(filename: str, data: dict) -> None:
def begin_batch(filename: str) -> None: def begin_batch(filename: str) -> None:
"""Defer tracker disk writes for filename. All _save calls accumulate in the """Defer tracker disk writes for filename. All _save calls accumulate in the
in-memory cache until flush_batch() is called. Use around per-person upload loops in-memory cache until flush_batch() is called. Use around per-person upload loops
to reduce N writes to 1.""" to reduce N writes to 1.
_deferred.add(str(_tracker_path(filename)))
If a previous batch for this file was interrupted before flush_batch() was called
(e.g. an exception escaped the upload loop), the leftover cache state is flushed
to disk here before starting fresh so that partial progress is not silently lost.
"""
path = _tracker_path(filename)
key = str(path)
if key in _deferred and key in _dirty:
try:
_write_to_disk(path, _cache[key])
except Exception:
logger.warning("begin_batch: could not flush leftover deferred state for %s — partial progress may be lost", path)
_deferred.discard(key)
_dirty.discard(key)
_deferred.add(key)
def flush_batch(filename: str) -> None: def flush_batch(filename: str) -> None:
"""Write the accumulated cache state for filename to disk.""" """Write the accumulated cache state for filename to disk."""
path = _tracker_path(filename) path = _tracker_path(filename)
key = str(path) key = str(path)
_deferred.discard(key) if key in _dirty and key in _cache:
if key in _cache:
_write_to_disk(path, _cache[key]) _write_to_disk(path, _cache[key])
_deferred.discard(key)
_dirty.discard(key)
def _flat_key(filename: str) -> str: def _flat_key(filename: str) -> str:
@@ -141,21 +158,24 @@ def _mark(
crop_dims: tuple[int, int] | None = None, crop_dims: tuple[int, int] | None = None,
frigate_score: float | None = None, frigate_score: float | None = None,
) -> None: ) -> None:
if not person_name:
logger.warning("_mark called with empty person_name for asset %s — asset not recorded", asset_id)
return
data = _load(filename) data = _load(filename)
if person_name: by_person = data.setdefault("by_person", {})
by_person = data.setdefault("by_person", {}) entry = _migrate_entry(by_person.get(person_name, {}))
entry = _migrate_entry(by_person.get(person_name, {})) ids = set(entry["asset_ids"])
ids = set(entry["asset_ids"]) ids.add(asset_id)
ids.add(asset_id) entry["asset_ids"] = sorted(ids)
entry["asset_ids"] = sorted(ids) if score is not None:
if score is not None: entry["scores"][asset_id] = round(score, 4)
entry["scores"][asset_id] = round(score, 4) if crop_dims is not None:
if crop_dims is not None: entry["crop_dims"][asset_id] = [crop_dims[0], crop_dims[1]]
entry["crop_dims"][asset_id] = [crop_dims[0], crop_dims[1]] if frigate_score is not None:
if frigate_score is not None: entry["frigate_scores"][asset_id] = round(frigate_score, 4)
entry["frigate_scores"][asset_id] = round(frigate_score, 4) by_person[person_name] = entry
by_person[person_name] = entry
_save(filename, data) _save(filename, data)
logger.debug("Marked %s in %s (%s)", asset_id, filename, person_name)
# ── Public API ──────────────────────────────────────────────────────────────── # ── Public API ────────────────────────────────────────────────────────────────
@@ -186,12 +206,10 @@ def mark_uploaded(
frigate_score: float | None = None, frigate_score: float | None = None,
) -> None: ) -> None:
_mark(UPLOAD_TRACKER_FILE, asset_id, person_name, score=score, crop_dims=crop_dims, frigate_score=frigate_score) _mark(UPLOAD_TRACKER_FILE, asset_id, person_name, score=score, crop_dims=crop_dims, frigate_score=frigate_score)
logger.debug(f"Marked {asset_id} as uploaded ({person_name})")
def mark_rejected(asset_id: str, person_name: str | None = None) -> None: def mark_rejected(asset_id: str, person_name: str | None = None) -> None:
_mark(REJECT_TRACKER_FILE, asset_id, person_name) _mark(REJECT_TRACKER_FILE, asset_id, person_name)
logger.debug(f"Marked {asset_id} as rejected ({person_name})")
@@ -205,11 +223,13 @@ def record_frigate_files_batch(person_name: str, mappings: dict[str, str]) -> No
"""Record multiple Frigate filename → asset_id mappings in a single load/save.""" """Record multiple Frigate filename → asset_id mappings in a single load/save."""
if not mappings: if not mappings:
return return
data = _load(UPLOAD_TRACKER_FILE) src = _load(UPLOAD_TRACKER_FILE)
by_person = data.setdefault("by_person", {}) by_person = dict(src.get("by_person", {}))
entry = _migrate_entry(by_person.get(person_name, {})) entry = _migrate_entry(by_person.get(person_name, {}))
entry["frigate_files"].update(mappings) entry["frigate_files"].update(mappings)
by_person[person_name] = entry by_person[person_name] = entry
data = dict(src)
data["by_person"] = by_person
_save(UPLOAD_TRACKER_FILE, data) _save(UPLOAD_TRACKER_FILE, data)
logger.debug(f"Batch-mapped {len(mappings)} Frigate file(s) for {person_name}") logger.debug(f"Batch-mapped {len(mappings)} Frigate file(s) for {person_name}")
@@ -225,17 +245,19 @@ def remove_frigate_file(person_name: str, frigate_filename: str) -> None:
def remove_frigate_files_batch(person_name: str, frigate_filenames: list[str]) -> None: def remove_frigate_files_batch(person_name: str, frigate_filenames: list[str]) -> None:
"""Remove multiple Frigate filenames in a single load/save.""" """Remove multiple Frigate filenames in a single load/save."""
data = _load(UPLOAD_TRACKER_FILE) src = _load(UPLOAD_TRACKER_FILE)
by_person = data.get("by_person", {}) raw = src.get("by_person", {}).get(person_name)
raw = by_person.get(person_name)
if raw is None: if raw is None:
return return
entry = _migrate_entry(raw) entry = _migrate_entry(raw)
for fn in frigate_filenames: for fn in frigate_filenames:
asset_id = entry["frigate_files"].pop(fn, None) asset_id = entry["frigate_files"].pop(fn, None)
if asset_id: if asset_id is not None and asset_id not in entry["frigate_files"].values():
entry["frigate_scores"].pop(asset_id, None) entry["frigate_scores"].pop(asset_id, None)
by_person = dict(src.get("by_person", {})) # copy so assignment does not mutate the cache
by_person[person_name] = entry by_person[person_name] = entry
data = dict(src)
data["by_person"] = by_person
_save(UPLOAD_TRACKER_FILE, data) _save(UPLOAD_TRACKER_FILE, data)
logger.debug(f"Removed {len(frigate_filenames)} Frigate file mapping(s) for {person_name}") logger.debug(f"Removed {len(frigate_filenames)} Frigate file mapping(s) for {person_name}")
@@ -265,9 +287,11 @@ def get_tracked_frigate_filenames(person_name: str) -> set[str]:
def has_frigate_scores(person_name: str) -> bool: def has_frigate_scores(person_name: str) -> bool:
"""Return True if any mapped file for this person has a stored Frigate recognition score.""" """Return True if any mapped file for this person has a stored Frigate recognition score."""
data = _load(UPLOAD_TRACKER_FILE) data = _load(UPLOAD_TRACKER_FILE)
entry = _migrate_entry(data.get("by_person", {}).get(person_name, {})) raw = data.get("by_person", {}).get(person_name)
frigate_files = entry.get("frigate_files", {}) if not raw or isinstance(raw, list):
frigate_scores = entry.get("frigate_scores", {}) return False
frigate_files = raw.get("frigate_files", {})
frigate_scores = raw.get("frigate_scores", {})
return any(asset_id in frigate_scores for asset_id in frigate_files.values()) return any(asset_id in frigate_scores for asset_id in frigate_files.values())
@@ -361,17 +385,19 @@ def reset_all_people() -> None:
approach is O(P²) because each call rebuilds the flat list from all remaining entries. approach is O(P²) because each call rebuilds the flat list from all remaining entries.
""" """
upload_data = _load(UPLOAD_TRACKER_FILE) upload_data = _load(UPLOAD_TRACKER_FILE)
frigate_url = _get_frigate_url()
if not frigate_url:
logger.info("FRIGATE_URL not set — skipping Frigate file deletion")
for person_name, raw_entry in upload_data.get("by_person", {}).items(): for person_name, raw_entry in upload_data.get("by_person", {}).items():
entry = _migrate_entry(raw_entry) entry = _migrate_entry(raw_entry)
frigate_filenames = list(entry.get("frigate_files", {}).keys()) frigate_filenames = list(entry.get("frigate_files", {}).keys())
if not frigate_filenames: if not frigate_filenames:
continue continue
if not os.environ.get("FRIGATE_URL", "").strip(): if frigate_url:
logger.info(f"FRIGATE_URL not set — skipping Frigate file deletion for {person_name}") if delete_frigate_person_files(person_name, frigate_filenames):
elif delete_frigate_person_files(person_name, frigate_filenames): logger.info(f"Deleted {len(frigate_filenames)} Frigate file(s) for {person_name}")
logger.info(f"Deleted {len(frigate_filenames)} Frigate file(s) for {person_name}") else:
else: logger.warning(f"Could not delete Frigate files for {person_name} — tracker reset proceeding anyway")
logger.warning(f"Could not delete Frigate files for {person_name} — tracker reset proceeding anyway")
_save(UPLOAD_TRACKER_FILE, {}) _save(UPLOAD_TRACKER_FILE, {})
_save(REJECT_TRACKER_FILE, {}) _save(REJECT_TRACKER_FILE, {})
logger.info("Reset all tracking data") logger.info("Reset all tracking data")
@@ -389,7 +415,7 @@ def reset_person(person_name: str) -> None:
entry = _migrate_entry(upload_data.get("by_person", {}).get(person_name, {})) entry = _migrate_entry(upload_data.get("by_person", {}).get(person_name, {}))
frigate_filenames = list(entry.get("frigate_files", {}).keys()) frigate_filenames = list(entry.get("frigate_files", {}).keys())
if frigate_filenames: if frigate_filenames:
if not os.environ.get("FRIGATE_URL", "").strip(): if not _get_frigate_url():
logger.info(f"FRIGATE_URL not set — skipping Frigate file deletion for {person_name}") logger.info(f"FRIGATE_URL not set — skipping Frigate file deletion for {person_name}")
elif delete_frigate_person_files(person_name, frigate_filenames): elif delete_frigate_person_files(person_name, frigate_filenames):
logger.info(f"Deleted {len(frigate_filenames)} Frigate file(s) for {person_name}") logger.info(f"Deleted {len(frigate_filenames)} Frigate file(s) for {person_name}")
@@ -398,15 +424,16 @@ def reset_person(person_name: str) -> None:
changed = False changed = False
for filename in (UPLOAD_TRACKER_FILE, REJECT_TRACKER_FILE): for filename in (UPLOAD_TRACKER_FILE, REJECT_TRACKER_FILE):
data = upload_data if filename == UPLOAD_TRACKER_FILE else _load(REJECT_TRACKER_FILE) src = upload_data if filename == UPLOAD_TRACKER_FILE else _load(REJECT_TRACKER_FILE)
by_person = data.get("by_person", {}) by_person = dict(src.get("by_person", {})) # copy so pop() does not mutate the cache
tracker_entry = by_person.pop(person_name, None) tracker_entry = by_person.pop(person_name, None)
if tracker_entry is not None: if tracker_entry is not None:
data = dict(src)
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:
data[flat_key] = sorted(set(data[flat_key]) - person_ids) data[flat_key] = sorted(set(data[flat_key]) - person_ids)
data["by_person"] = by_person
_save(filename, data) _save(filename, data)
changed = True changed = True
if changed: if changed: