flan/winnow
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address 10 full-codebase audit findings + lint

Browse Source 
Correctness:
- jobs: cap auto-diversity limit for brand-new people (was never capped,
  could exceed MAX_AUTO_IMAGES on first run)
- image_processing: separate None/0 guard for imageWidth/imageHeight so
  missing field is explicit rather than silently aliased to img_w
- upload_tracker (_mark, update_frigate_count): copy-before-mutate so
  exceptions between cache access and _save don't corrupt in-process state
- jobs: reject LIMIT=0 on no-embedding path (was silently empty run)
- jobs: add STRATEGY=skip to strategy_map so env var is honoured
- embeddings: convert to RGB before cvtColor so RGBA/grayscale thumbnails
  don't raise cv2.error and silently drop from diversity selection
- config: use falsy guard for OUTPUT_DIR so blank env var falls through
  to config file value
- reconcile: _ts() returns float("inf") on parse failure so unrecognised
  filenames sort last instead of collapsing to 0.0 and corrupting FIFO mapping
- diversity: remove dead selected_set (never read; -np.inf sentinel already
  prevents re-selection)

Lint (ruff):
- executor: sort upload_tracker import block (I001)
- executor: replace lambda is_better_than with operator.lt/gt (E731 x2)
- executor, upload_tracker: wrap long logger.warning calls (E501 x4)
This commit is contained in:
flan
2026-06-16 18:40:13 +00:00
parent 7a268d1ea2
commit 34fccf8839
8 changed files with 53 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
winnow/config.py
+1 -1
View File
@@ -173,7 +173,7 @@ class _Config:
data = json.loads(config_file.read_text())
if not self.IMMICH_URL:
self.IMMICH_URL = data.get("IMMICH_URL")
if os.getenv("OUTPUT_DIR") is None:
if not os.getenv("OUTPUT_DIR"):
self.OUTPUT_DIR = data.get("OUTPUT_DIR", self.OUTPUT_DIR)
except (json.JSONDecodeError, OSError) as e:
logging.warning("Failed to load config file: %s", e)
winnow/diversity.py
-2
View File
@@ -508,7 +508,6 @@ def _cluster_aware_selection(
medoid_indices, cluster_labels = _kmedoids(dist_matrix, k)
selected = list(medoid_indices)
selected_set = set(selected)
logger.debug("Selected %s cluster medoids as initial picks.", len(selected))
@@ -541,7 +540,6 @@ def _cluster_aware_selection(
break
selected.append(best_idx)
selected_set.add(best_idx)
# Update min distances
dists_to_new = dist_matrix[best_idx]
winnow/embeddings.py
+3 -2
View File
@@ -181,8 +181,9 @@ def get_face_embedding(img_pil: Image.Image) -> np.ndarray | None:
return None
try:
# InsightFace expects BGR cv2 image
img_bgr = cv2.cvtColor(np.asarray(img_pil), cv2.COLOR_RGB2BGR)
# InsightFace expects BGR cv2 image; normalise mode first so RGBA/grayscale don't
# raise a channel-count error inside cvtColor.
img_bgr = cv2.cvtColor(np.asarray(img_pil.convert("RGB")), cv2.COLOR_RGB2BGR)
# Suppress scikit-image FutureWarning from InsightFace's face_align.py
with warnings.catch_warnings():
winnow/executor.py
+20 -8
View File
@@ -1,6 +1,7 @@
"""Execution phase: image processing and Frigate upload."""
import logging
import operator
import os
import shutil
from io import BytesIO
@@ -27,8 +28,10 @@ from .log_config import console
from .quality import blur_score_from_image
from .reconcile import enrich_asset_with_face_data, reconcile_frigate_mappings
from .upload_tracker import (
UPLOAD_TRACKER_FILE,
REJECT_TRACKER_FILE,
UPLOAD_TRACKER_FILE,
begin_batch,
flush_batch,
get_lowest_quality_mapped_file,
get_most_redundant_mapped_file,
get_tracked_frigate_file_count,
@@ -36,8 +39,6 @@ from .upload_tracker import (
has_frigate_scores,
mark_rejected,
mark_uploaded,
begin_batch,
flush_batch,
remove_frigate_file,
remove_frigate_files_batch,
)
@@ -452,13 +453,13 @@ def upload_to_frigate(jobs: list[dict]) -> None:
get_target = get_most_redundant_mapped_file
score_label, better_note = "frigate", " (more novel)"
no_score_msg = "Frigate recognize unavailable, skipping replacement"
is_better_than = lambda c, t: c < t
is_better_than = operator.lt
else:
candidate_score = score_map.get(fname)
get_target = get_lowest_quality_mapped_file
score_label, better_note = "blur", ""
no_score_msg = "no quality score, skipping replacement"
is_better_than = lambda c, t: c > t
is_better_than = operator.gt
if candidate_score is None:
progress.console.print(f" [dim]⏭ {fname}: {no_score_msg}[/dim]")
@@ -489,7 +490,10 @@ def upload_to_frigate(jobs: list[dict]) -> None:
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)
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
@@ -603,11 +607,19 @@ def upload_to_frigate(jobs: list[dict]) -> None:
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)
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)
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.
if actually_uploaded and not _skip_reconcile:
winnow/image_processing.py
+7 -4
View File
@@ -92,11 +92,14 @@ def process_face_mode(
return None
img_w, img_h = img.size
meta_w = face_info.get("imageWidth") or img_w
meta_h = face_info.get("imageHeight") or img_h
meta_w = face_info.get("imageWidth") or 0
meta_h = face_info.get("imageHeight") or 0
# Scale bounding box to actual image dimensions
scale_x, scale_y = img_w / meta_w, img_h / meta_h
# Scale bounding box from detection-image space to actual image dimensions.
# Fall back to 1.0 if Immich omits the field — bbox is assumed to already
# be in image space (correct for thumbnails, wrong for full-res).
scale_x = img_w / meta_w if meta_w else 1.0
scale_y = img_h / meta_h if meta_h else 1.0
x1 = face_info["boundingBoxX1"] * scale_x
y1 = face_info["boundingBoxY1"] * scale_y
x2 = face_info["boundingBoxX2"] * scale_x
winnow/jobs.py
+8 -4
View File
@@ -66,7 +66,11 @@ def _get_strategy_choice(has_embedding: bool) -> tuple[int | str, str]:
def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, str]:
"""Resolve env var strategy to (limit, selection_mode) without prompts."""
if not has_embedding:
return _getenv_int("LIMIT", 30), "time"
limit = _getenv_int("LIMIT", 30)
if limit <= 0:
logger.warning("LIMIT=%s is invalid — ignoring and using default 30", limit)
limit = 30
return limit, "time"
custom_limit = _getenv_optional_int("LIMIT")
if custom_limit is not None:
@@ -77,6 +81,7 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st
strategy_map = {
"adaptive": ("auto", "smart"),
"auto": ("auto", "smart"), # legacy alias for adaptive
"skip": (0, "skip"),
"standard": (30, "smart"),
"broad": (100, "smart"),
}
@@ -297,9 +302,8 @@ def auto_configure(people: list[dict]) -> list[dict]:
if limit == "auto":
# Switch from open-ended auto to a fixed budget at remaining capacity
# so the diversity selector itself stops at the right count instead of
# selecting MAX_AUTO_IMAGES and then discarding the excess by position.
if already_uploaded > 0:
limit = capacity
# selecting more than MAX_AUTO_IMAGES and overflowing the cap.
limit = capacity
else:
limit = min(limit, capacity)
winnow/reconcile.py
+1 -1
View File
@@ -61,7 +61,7 @@ def reconcile_frigate_mappings(
try:
return float(fname.rsplit("_", 1)[-1].rsplit(".", 1)[0])
except (ValueError, IndexError):
return 0.0
return float("inf")
logger.debug(
"%s: mapping %s file(s) by filename timestamp — assumes Frigate processes"
winnow/upload_tracker.py
+13 -5
View File
@@ -109,7 +109,11 @@ def begin_batch(filename: str) -> None:
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)
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)
@@ -162,7 +166,7 @@ def _mark(
logger.warning("_mark called with empty person_name for asset %s — asset not recorded", asset_id)
return
data = _load(filename)
by_person = data.setdefault("by_person", {})
by_person = dict(data.get("by_person", {}))
entry = _migrate_entry(by_person.get(person_name, {}))
ids = set(entry["asset_ids"])
ids.add(asset_id)
@@ -174,7 +178,9 @@ def _mark(
if frigate_score is not None:
entry["frigate_scores"][asset_id] = round(frigate_score, 4)
by_person[person_name] = entry
_save(filename, data)
new_data = dict(data)
new_data["by_person"] = by_person
_save(filename, new_data)
logger.debug("Marked %s in %s (%s)", asset_id, filename, person_name)
@@ -371,11 +377,13 @@ def find_by_crop_dimension(size: int) -> list[dict]:
def update_frigate_count(person_name: str, count: int) -> None:
"""Record Frigate's authoritative training image count for a person."""
data = _load(UPLOAD_TRACKER_FILE)
by_person = data.setdefault("by_person", {})
by_person = dict(data.get("by_person", {}))
entry = _migrate_entry(by_person.get(person_name, {}))
entry["frigate_count"] = count
by_person[person_name] = entry
_save(UPLOAD_TRACKER_FILE, data)
new_data = dict(data)
new_data["by_person"] = by_person
_save(UPLOAD_TRACKER_FILE, new_data)
def reset_all_people() -> None: