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.

CHANGELOG.md

@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [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
 
 ### Changed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 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."
 license = "AGPL-3.0-or-later"
 requires-python = ">=3.13"

winnow/executor.py

@@ -510,7 +510,6 @@ def upload_to_frigate(jobs: list[dict]) -> None:
 
                                 asset_id = asset_map.get(fname)
                                 if asset_id:
-                                    tracker_ok = True
                                     try:
                                         mark_uploaded(
                                             asset_id,
@@ -520,7 +519,6 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                                             frigate_score=pre_fscore,
                                         )
                                     except Exception as tracker_exc:
-                                        tracker_ok = False
                                         # Upload to Frigate succeeded — don't retry on tracker
                                         # failure or we'd upload a duplicate to Frigate.
                                         logger.error(
@@ -528,7 +526,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                                             " but asset may be re-selected next run: %s",
                                             fname, tracker_exc,
                                         )
-                                    if tracker_ok:
+                                    else:
                                         if pre_fscore is not None:
                                             person_has_fscores = True
                                         actually_uploaded.append((fname, asset_id))

winnow/jobs.py

@@ -69,9 +69,9 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st
         return _getenv_int("LIMIT", 30), "time"
 
     custom_limit = _getenv_optional_int("LIMIT")
-    if custom_limit is not None and custom_limit > 0:
+    if custom_limit is not None:
+        if custom_limit > 0:
             return custom_limit, "smart"
-    if custom_limit is not None and custom_limit <= 0:
         logger.warning("LIMIT=%s is invalid — ignoring and using auto strategy", custom_limit)
 
     strategy_map = {

winnow/upload_tracker.py

@@ -223,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."""
     if not mappings:
         return
-    data = _load(UPLOAD_TRACKER_FILE)
-    by_person = data.setdefault("by_person", {})
+    src = _load(UPLOAD_TRACKER_FILE)
+    by_person = dict(src.get("by_person", {}))
     entry = _migrate_entry(by_person.get(person_name, {}))
     entry["frigate_files"].update(mappings)
     by_person[person_name] = entry
+    data = dict(src)
+    data["by_person"] = by_person
     _save(UPLOAD_TRACKER_FILE, data)
     logger.debug(f"Batch-mapped {len(mappings)} Frigate file(s) for {person_name}")