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

fix: address 10 code review findings (round 3)

Browse Source 
- diversity: scale face bbox to thumbnail space before quality check so
  check_face_size uses actual thumbnail pixels, not original-image coords
- diversity: skip asset when face bbox exists but crop guard rejects it,
  preventing InsightFace from picking the wrong person in a group photo
- diversity: add _scale_bbox_to_thumbnail helper (extracted from crop logic)
- diversity: use set for medoid membership test in _kmedoids (O(n) not O(n*k))
- diversity: remove dead np.unique in _select_time_spread (linspace produces
  strictly increasing indices; unique is a no-op and implies wrong semantics)
- embeddings: move os.open/os.dup calls inside try in _suppress_output so
  EMFILE during setup does not leak already-allocated fds
- immich_api: count and log assets with missing/unparseable fileCreatedAt in
  filter_recent_assets instead of silently discarding them
- executor: capture pre_run_count before stale-mapping cleanup so the
  "first run" coaching message doesn't fire after manual file deletion
- cli: use p['id'] (KeyError-safe) instead of p.get('id') in fallback path
  to match all other access sites on the same people list
- cache: narrow except to (OSError, ValueError) in EmbeddingCache.get so
  MemoryError propagates instead of converting OOM to a silent cache miss
This commit is contained in:
flan
2026-06-16 21:17:31 +00:00
parent 8bdce9253a
commit 4af9da2550
7 changed files with 62 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
winnow/cache.py
+1 -1
View File
@@ -75,7 +75,7 @@ class EmbeddingCache:
if os.path.exists(path):
try:
return np.load(path)
except Exception:
except (OSError, ValueError):
return None
return None
winnow/cli.py
+1 -1
View File
@@ -136,7 +136,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
"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]
return [p for p in people if p["id"] not in skip_ids]
# 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 from groups that merged successfully are already gone from Immich, so
winnow/diversity.py
+36 -3
View File
@@ -181,6 +181,28 @@ def _crop_face_from_thumbnail(
return crop
def _scale_bbox_to_thumbnail(
bbox: tuple[float, float, float, float],
img: Image.Image,
asset: dict,
person_id: str | None = None,
) -> tuple[float, float, float, float]:
"""Scale a face bbox from original detection-image space to thumbnail-pixel space."""
x1, y1, x2, y2 = bbox
img_w, img_h = img.size
for person in asset.get("people", []):
if person_id and person.get("id") != person_id:
continue
faces = person.get("faces", [])
if faces:
meta_w = faces[0].get("imageWidth") or img_w
meta_h = faces[0].get("imageHeight") or img_h
scale_x, scale_y = img_w / meta_w, img_h / meta_h
return (x1 * scale_x, y1 * scale_y, x2 * scale_x, y2 * scale_y)
break
return bbox
# =============================================================================
# Embedding Collection
# =============================================================================
@@ -258,9 +280,13 @@ def _select_by_embedding(
confidence = _get_face_confidence(asset, person_id=person_id)
face_bbox = _get_face_bbox(asset, person_id=person_id)
thumbnail_bbox = (
_scale_bbox_to_thumbnail(face_bbox, img, asset, person_id)
if face_bbox is not None else None
)
quality = assess_quality(
img,
face_bbox=face_bbox,
face_bbox=thumbnail_bbox,
confidence=confidence,
blur_threshold=Config.BLUR_THRESHOLD,
min_face_px=Config.MIN_FACE_WIDTH,
@@ -273,6 +299,12 @@ def _select_by_embedding(
asset["quality_score"] = quality.blur_score
face_crop = _crop_face_from_thumbnail(img, asset, person_id=person_id)
if face_crop is None and face_bbox is not None:
logger.warning(
"Face too small to crop for %s — skipping to avoid embedding wrong person",
asset["id"],
)
continue
embed_img = face_crop if face_crop is not None else img
emb = get_embedding(embed_img, asset_id=asset["id"])
@@ -408,7 +440,8 @@ def _kmedoids(dist_matrix: np.ndarray, k: int, max_iter: int = 50) -> tuple[list
for _ in range(max_iter):
improved = False
# Try swapping each medoid with a random non-medoid
non_medoids = [i for i in range(n) if i not in medoids]
medoid_set = set(medoids)
non_medoids = [i for i in range(n) if i not in medoid_set]
if not non_medoids:
break
@@ -576,4 +609,4 @@ def _select_time_spread(assets: list, limit: int | str) -> list:
return assets
indices = np.linspace(0, len(assets) - 1, limit, dtype=int)
return [assets[i] for i in np.unique(indices)]
return [assets[i] for i in indices]
winnow/embeddings.py
+13 -10
View File
@@ -26,22 +26,25 @@ logger = logging.getLogger(__name__)
@contextmanager
def _suppress_output():
"""Suppress stdout/stderr at the file-descriptor level, silencing C extension noise."""
devnull_fd = os.open(os.devnull, os.O_WRONLY)
saved_out, saved_err = os.dup(1), os.dup(2)
devnull_fd = None
saved_out = None
saved_err = None
try:
devnull_fd = os.open(os.devnull, os.O_WRONLY)
saved_out = os.dup(1)
saved_err = os.dup(2)
os.dup2(devnull_fd, 1)
os.dup2(devnull_fd, 2)
yield
finally:
try:
if saved_out is not None:
os.dup2(saved_out, 1)
finally:
try:
os.dup2(saved_err, 2)
finally:
os.close(devnull_fd)
os.close(saved_out)
os.close(saved_err)
os.close(saved_out)
if saved_err is not None:
os.dup2(saved_err, 2)
os.close(saved_err)
if devnull_fd is not None:
os.close(devnull_fd)
# Lazy-loaded singleton
winnow/executor.py
+1 -1
View File
@@ -327,6 +327,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
# TODO(frigate-api): if Frigate exposes per-file embeddings, compute
# diversity against the full training set (tracked + manual) rather than
# relying solely on the Frigate score as a proxy signal.
pre_run_count = get_tracked_frigate_file_count(name)
_snapshot = (
all_frigate_files.get(name, []) if all_frigate_files is not None
else get_frigate_person_files(name)
@@ -358,7 +359,6 @@ def upload_to_frigate(jobs: list[dict]) -> None:
" (file(s) no longer in Frigate)[/dim]"
)
effective_count = get_tracked_frigate_file_count(name)
pre_run_count = effective_count
quality_replacement = job.get("config", {}).get("quality_replacement", False)
if Config.ENABLE_FRIGATE_SCORES and pre_run_count == 0:
progress.console.print(
winnow/immich_api.py
+8 -1
View File
@@ -285,10 +285,11 @@ def filter_recent_assets(assets: list[dict], years: int | None = None) -> list[d
logger.debug("Filtering assets older than %s years (%s)", years, cutoff)
recent, skipped = [], 0
recent, skipped, bad_timestamp = [], 0, 0
for asset in assets:
created_at_str = asset.get("fileCreatedAt")
if not isinstance(created_at_str, str) or not created_at_str:
bad_timestamp += 1
continue
try:
@@ -299,8 +300,14 @@ def filter_recent_assets(assets: list[dict], years: int | None = None) -> list[d
else:
skipped += 1
except ValueError:
bad_timestamp += 1
continue
if bad_timestamp:
logger.warning(
"filter_recent_assets: %s asset(s) had missing or unparseable fileCreatedAt"
" and were excluded from the pool.", bad_timestamp
)
logger.debug("Retained %s assets (filtered %s old assets).", len(recent), skipped)
return recent
winnow/quality.py
+2 -1
View File
@@ -155,7 +155,8 @@ def blur_score_from_image(img: Image.Image, max_dim: int = 1440) -> float | None
try:
score_img = img.convert("RGB") if img.mode != "RGB" else img
if score_img.width > max_dim or score_img.height > max_dim:
score_img = score_img.copy()
if score_img is img:
score_img = score_img.copy()
score_img.thumbnail((max_dim, max_dim), Image.LANCZOS)
return _laplacian_var(np.array(score_img))
except Exception as exc: