diff --git a/CHANGELOG.md b/CHANGELOG.md index 276c536..58c90b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.6.4] - 2026-06-17 + +### Fixed + +- **Face bbox scaled to thumbnail space before quality filtering**: `assess_quality` now receives coordinates in thumbnail-pixel space rather than detection-image space. Previously, a face detected on a full-resolution image (e.g. 4000 px wide) was compared against `MIN_FACE_WIDTH` using its original pixel dimensions, causing faces that appear small on the thumbnail to pass the quality filter — and faces that appear large to be incorrectly rejected. + +- **`conf_array` default restored to 1.0 for faces with missing confidence**: the default was incorrectly set to 0.5, causing images with no `score` field in the Immich faces API response to receive a 1.7× FPS diversity boost and be selected ahead of genuinely high-confidence detections. The default is now 1.0 (no boost), treating missing confidence as neutral. + +- **`hard_weight` computed once outside FPS loop**: `conf_array` is constant after initialisation; moving the `np.where` call outside the `while` loop eliminates one O(n) numpy pass per selected image. + +- **`has_frigate_model` snapshot prevents mid-batch `recognize_face` calls on first run**: `effective_count` is incremented inside the upload loop, so using it as the `recognize_face` gate would incorrectly trigger scoring after the first upload on a first run. A boolean snapshot is now taken before the loop. + +- **`person_has_fscores` only set when tracker write succeeds**: the flag was moved outside the `try/except else` block, causing at-cap replacement to switch into Frigate-score mode even when the score was never written to the tracker — `get_most_redundant_mapped_file` then returned `None` and all replacement candidates were silently skipped. The flag is now set only in the `else` branch. + +- **`STRATEGY=skip` honoured before embedding and limit checks**: the strategy was silently converted to `auto` when InsightFace was available, because two early-returns in `_resolve_strategy` ran before the `strategy_map` lookup. + +- **`limit="auto"` preserved on first run**: switching to `limit = capacity` unconditionally caused the FPS adaptive early-stop to never fire on a person's first upload run. `limit="auto"` is now kept when `already_uploaded == 0`. + +- **`EmbeddingCache.get` falls back gracefully on all load errors**: a `MemoryError` during `np.load` of a cached embedding was re-raised, crashing the entire diversity-selection batch for that person. Cache-read failures of any kind now return `None` so the embedding is recomputed fresh. + +- **`get_people` returns `[]` when Immich sends `{"people": null}`**: `.get("people", [])` only uses the default when the key is absent, not when its value is `null`. Changed to `data.get("people") or []` so null-valued responses are handled the same as missing keys. + +- **`get_people` and `fetch_all_assets` guard against non-dict responses**: a proxy or CDN returning a JSON array (or other non-dict body) previously caused an `AttributeError` from `.get()`. Both functions now check `isinstance(data, dict)` and return an empty result with an error log. + +- **`filter_recent_assets` counts and logs assets with missing or unparseable timestamps** instead of silently dropping them. + +- **`_suppress_output` fd cleanup restructured**: the context manager now initialises `devnull_fd`, `saved_out`, and `saved_err` to `None` before the `try` block, so the `finally` can close only the descriptors that were successfully opened. Each `os.close` is wrapped in its own `try/except OSError` so a failed close cannot prevent subsequent descriptors from being released. `OSError` from `os.dup2` restore is logged at DEBUG rather than silently swallowed. + +- **`blur_score_from_image` copies the image before thumbnail resize**: `Image.thumbnail` modifies the image in-place. When the caller's image was already in RGB mode (no convert copy), the resize would have mutated the caller's object. A copy is now made when `score_img is img`. + +- **`imageWidth`/`imageHeight` zero-value treated as missing** in `image_processing.py`: the old `or img_w` fallback silently set `scale = 1.0` for a zero-valued dimension (correct) but also for `None` (also correct) with no distinction. The explicit `scale = img_w / meta_w if meta_w else 1.0` form matches the pattern used in the new `_scale_bbox_to_thumbnail` helper and makes the fallback intent clear. + +- **`_mark` and `update_frigate_count` copy before mutate**: both functions now create a shallow copy of the top-level tracker dict before assigning into `by_person`, so a failed `_save` cannot leave the in-memory cache ahead of the on-disk file. + +- **`reset_person` flat-list guard only warns when cleanup would have run**: the `isinstance(data[flat_key], list)` check previously emitted a warning even when `person_ids` was empty (a no-op call). The warning is now gated behind `person_ids and`, matching the guard on the cleanup branch. + +- **`_handle_duplicate_people` uses `p.get("id")` consistently**: all four return-path filter comprehensions and the `_smaller_duplicate_ids` set comprehension now use `.get("id")` instead of bare `p["id"]`, preventing a `KeyError` if the Immich API returns a person record without an `id` field. + +- **`K-Medoids` non-medoid membership test is O(1)**: `non_medoids` now filters against `set(medoids)` instead of the list, eliminating an O(k) scan per candidate on each outer iteration. + ## [0.6.3] - 2026-06-16 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index a4437a0..3a82977 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "winnow" -version = "0.6.3" +version = "0.6.4" description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition." license = "AGPL-3.0-or-later" requires-python = ">=3.13" diff --git a/winnow/cache.py b/winnow/cache.py index 6ce3126..183c3a2 100644 --- a/winnow/cache.py +++ b/winnow/cache.py @@ -103,8 +103,11 @@ class EmbeddingCache: count = 0 for f in os.listdir(self.cache_dir): if f.endswith(".npy"): - os.remove(os.path.join(self.cache_dir, f)) - count += 1 + try: + os.remove(os.path.join(self.cache_dir, f)) + count += 1 + except OSError: + pass logger.info("Cleared %s cached embeddings.", count) diff --git a/winnow/cli.py b/winnow/cli.py index 2ce8d60..7ac5447 100644 --- a/winnow/cli.py +++ b/winnow/cli.py @@ -81,7 +81,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]: def _smaller_duplicate_ids(groups: dict) -> set[str]: """IDs of all but the largest person in each duplicate group.""" return { - p["id"] + p.get("id") for ps in groups.values() for p in sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)[1:] } @@ -109,7 +109,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]: ) # Return deduplicated list — keep only the largest per name so that # downstream job creation never runs two jobs for the same Frigate folder. - return [p for p in people if p["id"] not in skip_ids] + return [p for p in people if p.get("id") not in skip_ids] # Auto-merge: survivor = largest asset count, rest merge into it inside Immich merged_any = False @@ -131,6 +131,17 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]: if merged_any: rprint(" [dim]Re-fetching people after merge...[/dim]") fresh = get_people() + if not fresh: + # Retry once: get_people() returns [] for both transient failures and + # auth errors (401); a second empty result strongly suggests a real failure. + fresh = get_people() + if not fresh: + logger.warning( + "Re-fetch after merge returned no people (tried twice)" + " — possible transient error or expired API key;" + " proceeding with pre-merge list. Check IMMICH_API_KEY if this recurs." + ) + return [p for p in people if p.get("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 @@ -143,7 +154,7 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]: " [yellow]All merges failed — applying local deduplication" " to avoid overwriting output.[/yellow]" ) - return [p for p in people if p["id"] not in skip_ids] + return [p for p in people if p.get("id") not in skip_ids] _UNSUPPORTED_VARS = [ @@ -173,7 +184,8 @@ def main() -> None: [dim]Immich -> Frigate Training Data Curator[/dim] """) - set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v)] + _FALSY = {"", "false", "0", "no", "off"} + set_unsupported = [v for v in _UNSUPPORTED_VARS if os.environ.get(v, "").strip().lower() not in _FALSY] if set_unsupported: console.print( f"[bold yellow]⚠ Advanced tuning vars set: " diff --git a/winnow/config.py b/winnow/config.py index 37a99c7..2a33680 100644 --- a/winnow/config.py +++ b/winnow/config.py @@ -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) diff --git a/winnow/diversity.py b/winnow/diversity.py index c412743..2b55c2e 100644 --- a/winnow/diversity.py +++ b/winnow/diversity.py @@ -57,9 +57,9 @@ def select_diverse_assets( Returns: List of selected assets """ - # Fast path: fewer assets than limit + # Fast path: fewer assets than limit — sort for consistent ordering with other paths if limit != "auto" and len(assets) <= limit: - return assets + return sorted(assets, key=lambda x: x.get("fileCreatedAt", "")) # Sort by creation time assets = sorted(assets, key=lambda x: x.get("fileCreatedAt", "")) @@ -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 0 + meta_h = faces[0].get("imageHeight") or 0 + scale_x = img_w / meta_w if meta_w else 1.0 + scale_y = img_h / meta_h if meta_h else 1.0 + return (x1 * scale_x, y1 * scale_y, x2 * scale_x, y2 * scale_y) + 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, @@ -408,7 +434,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 @@ -483,7 +510,10 @@ def _cluster_aware_selection( norms = np.linalg.norm(emb_matrix, axis=1, keepdims=True) emb_normed = emb_matrix / np.maximum(norms, 1e-8) - # Build confidence weight array for hard example boosting + # Build confidence weight array for hard example boosting. + # Default to 1.0 for faces with no confidence score: treat as high-confidence + # (no boost) rather than hard-example territory. A missing score field should + # not cause these images to beat genuinely high-confidence detections in FPS. conf_array = np.ones(n) if confidence_scores: for i, c in enumerate(confidence_scores): @@ -508,7 +538,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)) @@ -522,10 +551,11 @@ def _cluster_aware_selection( for idx in selected: min_dists[idx] = -np.inf + # Hard example weighting: boost distance for low-confidence candidates. + # conf_array is constant after this point, so compute once outside the loop. + hard_weight = np.where(conf_array < 0.85, 1.0 + (0.85 - conf_array) * 2.0, 1.0) + while len(selected) < target: - # Hard example weighting: boost distance for low-confidence candidates - # Confidence < 0.85 gets up to 1.5× distance boost - hard_weight = np.where(conf_array < 0.85, 1.0 + (0.85 - conf_array) * 2.0, 1.0) weighted_dists = min_dists * hard_weight best_idx = int(np.argmax(weighted_dists)) @@ -541,15 +571,19 @@ def _cluster_aware_selection( break selected.append(best_idx) - selected_set.add(best_idx) # Update min distances dists_to_new = dist_matrix[best_idx] min_dists = np.minimum(min_dists, dists_to_new) min_dists[best_idx] = -np.inf - selected_conf = [conf_array[i] for i in selected if conf_array[i] < 1.0] - hard_count = sum(1 for c in selected_conf if c < 0.85) + hard_count = sum( + 1 for i in selected + if confidence_scores + and i < len(confidence_scores) + and confidence_scores[i] is not None + and confidence_scores[i] < 0.85 + ) logger.info("Selection complete: %s images (%s hard examples with confidence < 0.85).", len(selected), hard_count) # Slice to target: the while loop enforces this for non-auto mode, but @@ -576,4 +610,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] diff --git a/winnow/embeddings.py b/winnow/embeddings.py index cab4294..a715162 100644 --- a/winnow/embeddings.py +++ b/winnow/embeddings.py @@ -26,22 +26,47 @@ 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: - os.dup2(saved_out, 1) - finally: + # Each block is a separate sequential statement. A BaseException (e.g. + # KeyboardInterrupt) raised inside block N would propagate past blocks N+1 + # and N+2, leaving saved_err or devnull_fd unclosed. In CPython, KI is + # delivered between bytecodes, not mid-syscall; os.dup2 is a single C call + # and completes atomically, so this race is not realistically triggerable. + if saved_out is not None: + try: + os.dup2(saved_out, 1) + except OSError as e: + logger.debug("_suppress_output: failed to restore stdout fd: %s", e) + finally: + try: + os.close(saved_out) + except OSError: + pass + if saved_err is not None: try: os.dup2(saved_err, 2) + except OSError as e: + logger.debug("_suppress_output: failed to restore stderr fd: %s", e) finally: + try: + os.close(saved_err) + except OSError: + pass + if devnull_fd is not None: + try: os.close(devnull_fd) - os.close(saved_out) - os.close(saved_err) + except OSError: + pass # Lazy-loaded singleton @@ -181,8 +206,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(): diff --git a/winnow/executor.py b/winnow/executor.py index c5fee48..52b7401 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -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, ) @@ -357,12 +358,16 @@ 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: + if Config.ENABLE_FRIGATE_SCORES and effective_count == 0: progress.console.print( f" [dim]{name}: first run — Frigate diversity scoring will apply from the next run[/dim]" ) + # Snapshot whether Frigate has a model before the upload loop starts. + # effective_count is incremented inside the loop on each successful upload, + # so using the live value would incorrectly trigger recognize_face calls + # mid-batch on the first run (after the first upload sets it to 1). + has_frigate_model = effective_count > 0 actually_uploaded: list[tuple[str, str | None]] = [] failed_deletes: set[str] = set() min_quality_score_for_slot: float | None = None @@ -391,8 +396,8 @@ def upload_to_frigate(jobs: list[dict]) -> None: # 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 - # 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. + # replacement) and for at-cap uploads when scores already exist. + # Skipped when has_frigate_model is False (effective_count was 0 before the loop). # 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 # as None so a wrong-person score never drives a ceiling skip or replacement. @@ -408,7 +413,7 @@ def upload_to_frigate(jobs: list[dict]) -> None: # rebuild-complete signal, poll it between recognize calls during replacement # sequences rather than accepting stale/None scores. pre_fscore: float | None = None - if Config.ENABLE_FRIGATE_SCORES and pre_run_count > 0: + if Config.ENABLE_FRIGATE_SCORES and has_frigate_model: if not at_cap or person_has_fscores: _result = recognize_face(fpath) if _result is not None and (_result[0] or "").casefold() == name.casefold(): @@ -416,8 +421,8 @@ def upload_to_frigate(jobs: list[dict]) -> None: # Below-cap novelty gate: skip candidates already covered by the Frigate model, # 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 - # above), so this block never fires on the first run without an extra guard. + # pre_fscore is None when effective_count == 0 (no Frigate model yet), + # so this block never fires on the first run without an extra guard. if not at_cap and pre_fscore is not None: _ceiling = Config.FRIGATE_SCORE_CEILING if _ceiling is None: @@ -452,13 +457,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 +494,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 @@ -529,7 +537,17 @@ def upload_to_frigate(jobs: list[dict]) -> None: else: if pre_fscore is not None: person_has_fscores = True - actually_uploaded.append((fname, asset_id)) + # Always record for reconcile so the Frigate filename→asset_id + # mapping is created even when the tracker write fails. + # Trade-off: if mark_uploaded failed, asset_id is absent from + # asset_ids and scores. Consequences: (1) re-selected next run + # → Frigate duplicate; (2) excluded from quality-replacement + # candidates (_pick_mapped_file requires a scores entry); + # (3) counted toward MAX_AUTO_IMAGES cap (via frigate_files). + # The alternative — not appending — leaves the file permanently + # unmapped (reconcile never creates the frigate_files entry), + # making (2) and (3) permanent. Frigate duplicate is lesser. + actually_uploaded.append((fname, asset_id)) break else: @@ -603,11 +621,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: diff --git a/winnow/image_processing.py b/winnow/image_processing.py index 7ded632..23c3f0e 100644 --- a/winnow/image_processing.py +++ b/winnow/image_processing.py @@ -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 diff --git a/winnow/immich_api.py b/winnow/immich_api.py index 3bde08b..6ecc31f 100644 --- a/winnow/immich_api.py +++ b/winnow/immich_api.py @@ -57,8 +57,12 @@ def get_people() -> list[dict]: logger.error("Immich API key is invalid or expired (401 Unauthorized). Update API_KEY.") return [] resp.raise_for_status() - return resp.json().get("people", []) - except (requests.RequestException, ValueError) as e: + data = resp.json() + if not isinstance(data, dict): + logger.error("Unexpected response shape from Immich /people: %r", type(data)) + return [] + return data.get("people") or [] + except (requests.RequestException, ValueError, AttributeError) as e: logger.error("Failed to fetch people from Immich: %s", e) return [] @@ -118,7 +122,11 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]: logger.error("Error fetching assets for %s (page %s): %s", name, page, resp.status_code) break - page_assets = resp.json().get("assets", []) + body = resp.json() + if not isinstance(body, dict): + logger.error("Unexpected response shape fetching assets for %s (page %s): %r", name, page, type(body)) + break + page_assets = body.get("assets", []) # Immich ≥2.x returns {"assets": {"items": [...]}}; # earlier versions returned {"assets": [...]} directly. if isinstance(page_assets, dict): @@ -277,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: @@ -291,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 diff --git a/winnow/jobs.py b/winnow/jobs.py index b8243c7..396fc7a 100644 --- a/winnow/jobs.py +++ b/winnow/jobs.py @@ -65,8 +65,14 @@ 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 strategy == "skip": + return 0, "skip" 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: @@ -295,10 +301,11 @@ def auto_configure(people: list[dict]) -> list[dict]: # decides per-image whether to swap; any candidate could be an improvement). if not quality_replacement_only: 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: + # Switch from open-ended auto to a fixed budget at remaining capacity + # so the diversity selector stops at the right count instead of + # selecting more than MAX_AUTO_IMAGES and overflowing the cap. + # First runs keep limit="auto" so FPS adaptive early-stop can fire. limit = capacity else: limit = min(limit, capacity) diff --git a/winnow/quality.py b/winnow/quality.py index 1becded..39f58c6 100644 --- a/winnow/quality.py +++ b/winnow/quality.py @@ -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: diff --git a/winnow/reconcile.py b/winnow/reconcile.py index 2a4f251..c012f87 100644 --- a/winnow/reconcile.py +++ b/winnow/reconcile.py @@ -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" diff --git a/winnow/upload_tracker.py b/winnow/upload_tracker.py index 3b56678..04ead4c 100644 --- a/winnow/upload_tracker.py +++ b/winnow/upload_tracker.py @@ -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) @@ -354,6 +360,8 @@ def find_by_crop_dimension(size: int) -> list[dict]: asset_to_frigate.setdefault(aid, fn) # first-seen wins; plain inversion silently drops duplicates frigate_scores = entry.get("frigate_scores", {}) for asset_id, dims in entry.get("crop_dims", {}).items(): + if not isinstance(dims, (list, tuple)) or len(dims) < 2: + continue w, h = dims[0], dims[1] if w == size or h == size: results.append({ @@ -371,11 +379,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: @@ -432,7 +442,13 @@ def reset_person(person_name: str) -> None: data["by_person"] = by_person flat_key = _flat_key(filename) person_ids = set(_get_ids(tracker_entry)) - if person_ids and flat_key in data: + if person_ids and flat_key in data and not isinstance(data[flat_key], list): + logger.warning( + "reset_person: %s has unexpected type for %s (%s) — skipping flat-list cleanup;" + " all persons' legacy IDs in this field are unaffected but unreadable", + filename, flat_key, type(data[flat_key]).__name__, + ) + elif person_ids and flat_key in data: data[flat_key] = sorted(set(data[flat_key]) - person_ids) _save(filename, data) changed = True