fix: v0.5.20 — migration safety, URL normalization, image rejection, atomic writes

- upload_tracker: replace executescript() in _migrate_schema_v2 with
  individual execute() calls inside a transaction so a crash between DROP
  and RENAME rolls back instead of permanently destroying tracked_assets
- frigate_api: _get_frigate_url now strips leading/trailing whitespace
  before rstrip('/') so whitespace-only FRIGATE_URL is treated as unset
- executor: upload_to_frigate now uses _get_frigate_url() eliminating
  double-slash upload paths when FRIGATE_URL has a trailing slash
- executor: corrupt thumbnail (resp.ok=True, Image.open fails) now calls
  mark_rejected() so permanently broken assets are not retried forever
- upload_tracker: reset_person now uses _get_frigate_url() instead of
  inline os.environ.get('FRIGATE_URL', '').strip()
- image_processing: _save_jpeg writes to a .tmp file and calls
  os.replace() so a disk-full error never leaves a truncated JPEG
- cli: _handle_duplicate_people falls back to local deduplication when
  all Immich merges fail, preventing two jobs from overwriting the same
  Frigate folder
- config: _getenv_optional_float now delegates to _getenv_num() like
  _getenv_optional_int, eliminating the inconsistent duplicate
- reconcile: _ts() uses rsplit('.', 1)[0] instead of .replace('.webp','')
  so FIFO mapping works with any Frigate training-file extension

CHANGELOG.md

@@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.5.20] - 2026-06-15
+
+### Fixed
+
+- **`_migrate_schema_v2` is now crash-safe**: the previous implementation used `conn.executescript()`, which issues an implicit `COMMIT` before executing — so a process kill between the `DROP TABLE` and the `ALTER TABLE RENAME` would permanently destroy `tracked_assets` with no rollback. Replaced with individual `conn.execute()` calls inside a `with conn:` transaction so the entire migration rolls back on failure.
+
+- **`FRIGATE_URL` with a trailing slash no longer produces double-slash upload paths**: `upload_to_frigate` in `executor.py` read `os.environ.get("FRIGATE_URL", "")` directly, bypassing the `.rstrip("/")` normalization in `frigate_api._get_frigate_url()`. A `FRIGATE_URL` ending in `/` produced paths like `/api/faces//Alice/register` for uploads while all other Frigate API calls used the cleaned URL. Both `executor.py` and `upload_tracker.reset_person` now call `_get_frigate_url()` instead of reading the env var inline.
+
+- **Corrupt thumbnail content now marks the asset rejected**: when `resp.ok=True` but `Image.open()` raises (corrupt JPEG bytes from Immich), the asset was silently skipped with no tracker entry, causing it to be re-selected and re-downloaded on every future run. The path now calls `mark_rejected()` so a permanently corrupt thumbnail doesn't cause an indefinite retry loop.
+
+- **`_save_jpeg` writes atomically**: the face crop JPEG was written directly to its final path — a disk-full or PIL encode error mid-write would leave a truncated file at the output path with no cleanup. The helper now writes to `{path}.tmp` and only calls `os.replace()` on success; on failure the temporary file is removed and the exception is re-raised.
+
+- **`_handle_duplicate_people` deduplicates even when all Immich merges fail**: when `MERGE_DUPLICATE_PEOPLE=true` and every `merge_people()` call returns `False`, the function previously returned the original unfiltered people list. Two jobs for the same person then ran sequentially, with the second job's `shutil.rmtree` wiping the first job's uploaded crops. The function now falls back to local deduplication (keep largest per name) whenever merging fails.
+
+- **`_get_frigate_url()` strips leading/trailing whitespace**: `os.environ.get("FRIGATE_URL", "").rstrip("/")` left whitespace-only values like `"   "` as truthy, allowing them to reach API calls as malformed URLs. Added `.strip()` before `.rstrip("/")` so a whitespace-only value collapses to the empty string and is treated as unset.
+
+### Changed
+
+- **`_getenv_optional_float` now delegates to `_getenv_num`**: the function hand-rolled its own strip/cast/warn/None logic instead of calling `_getenv_num(name, None, float)` the way `_getenv_optional_int` does. Both optional helpers are now consistent and pick up any future changes to the shared `_getenv_num` implementation automatically.
+
+- **`reconcile._ts()` strips any file extension, not just `.webp`**: the Frigate timestamp extracted from training filenames used `.replace(".webp", "")`, which silently returns `0.0` for any non-`.webp` filename and produces undefined-order FIFO mappings if Frigate ever changes its training-file extension. Replaced with `.rsplit(".", 1)[0]` to strip the last extension generically.
+
 ## [0.5.19] - 2026-06-15
 
 ### Fixed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "winnow"
-version = "0.5.19"
+version = "0.5.20"
 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/cli.py

@@ -127,7 +127,18 @@ def _handle_duplicate_people(people: list[dict]) -> list[dict]:
         rprint("  [dim]Re-fetching people after merge...[/dim]")
         return get_people()
 
-    return people
+    # All merges failed — fall back to local deduplication (keep largest per name) so
+    # downstream job creation never runs two jobs for the same Frigate folder.
+    rprint(
+        "  [yellow]All merges failed — applying local deduplication"
+        " to avoid overwriting output.[/yellow]"
+    )
+    skip_ids = {
+        p["id"]
+        for ps in duplicates.values()
+        for p in sorted(ps, key=lambda x: x.get("assetCount", 0), reverse=True)[1:]
+    }
+    return [p for p in people if p["id"] not in skip_ids]
 
 
 _UNSUPPORTED_VARS = [

winnow/config.py

@@ -35,14 +35,8 @@ def _getenv_float(name: str, default: float) -> float:
 
 
 def _getenv_optional_float(name: str) -> float | None:
-    raw = os.getenv(name, "").strip()
-    if not raw:
-        return None
-    try:
-        return float(raw)
-    except ValueError:
-        logging.warning("%s=%r is not a valid float — ignoring", name, raw)
-        return None
+    """Return float value of env var, or None if unset/empty. Warns and returns None on invalid."""
+    return _getenv_num(name, None, float)
 
 
 def _getenv_optional_int(name: str) -> int | None:

winnow/executor.py

@@ -13,6 +13,7 @@ from rich.progress import BarColumn, Progress, SpinnerColumn, TaskProgressColumn
 
 from .config import Config, get_headers
 from .frigate_api import (
+    _get_frigate_url,
     delete_frigate_person_files,
     get_all_frigate_person_files,
     get_frigate_person_files,
@@ -158,7 +159,11 @@ def execute_jobs(jobs: list[dict]) -> None:
                                 try:
                                     img = Image.open(BytesIO(resp.content))
                                 except Exception:
-                                    logger.warning("Invalid image data for asset %s", asset["id"])
+                                    # resp.ok=True but content is unreadable — corrupt Immich
+                                    # thumbnail. Mark rejected so this asset isn't retried
+                                    # indefinitely on future runs.
+                                    logger.warning("Invalid image data for asset %s — marking rejected", asset["id"])
+                                    mark_rejected(asset["id"], person_name=name)
                                     img = None
                             else:
                                 img = None
@@ -216,7 +221,7 @@ def upload_to_frigate(jobs: list[dict]) -> None:
         rprint("[dim]No jobs to upload.[/dim]")
         return
 
-    frigate_url = os.environ.get("FRIGATE_URL", "")
+    frigate_url = _get_frigate_url()
     if not frigate_url:
         rprint("[yellow]⚠️  FRIGATE_URL not set, skipping upload.[/yellow]")
         return

winnow/frigate_api.py

@@ -9,8 +9,8 @@ logger = logging.getLogger(__name__)
 
 
 def _get_frigate_url() -> str:
-    """Return normalized FRIGATE_URL with trailing slash stripped, or '' if unset."""
-    return os.environ.get("FRIGATE_URL", "").rstrip("/")
+    """Return normalized FRIGATE_URL with whitespace and trailing slash stripped, or '' if unset."""
+    return os.environ.get("FRIGATE_URL", "").strip().rstrip("/")
 
 
 def get_frigate_version() -> str | None:

winnow/image_processing.py

@@ -15,7 +15,16 @@ logger = logging.getLogger(__name__)
 def _save_jpeg(img: Image.Image, path: str) -> None:
     if img.mode != "RGB":
         img = img.convert("RGB")
-    img.save(path, format="JPEG")
+    tmp = path + ".tmp"
+    try:
+        img.save(tmp, format="JPEG")
+        os.replace(tmp, path)
+    except Exception:
+        try:
+            os.remove(tmp)
+        except OSError:
+            pass
+        raise
 
 
 def align_face(img: Image.Image, landmarks: list[list[float]] | np.ndarray) -> Image.Image | None:

winnow/reconcile.py

@@ -59,7 +59,7 @@ def reconcile_frigate_mappings(
     if len(new_files) == target:
         def _ts(fname: str) -> float:
             try:
-                return float(fname.rsplit("_", 1)[-1].replace(".webp", ""))
+                return float(fname.rsplit("_", 1)[-1].rsplit(".", 1)[0])
             except (ValueError, IndexError):
                 return 0.0
 

winnow/upload_tracker.py

@@ -20,7 +20,7 @@ import os
 import sqlite3
 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__)
 
@@ -76,23 +76,29 @@ def _migrate_schema_v2(conn: sqlite3.Connection) -> None:
         return  # Already at new schema
 
     logger.info("Migrating tracked_assets: adding person_name to primary key")
-    conn.executescript("""
-        CREATE TABLE tracked_assets_new (
-            asset_id      TEXT NOT NULL,
-            person_name   TEXT,
-            status        TEXT NOT NULL CHECK(status IN ('uploaded', 'rejected')),
-            blur_score    REAL,
-            crop_width    INTEGER,
-            crop_height   INTEGER,
-            frigate_score REAL,
-            PRIMARY KEY (asset_id, person_name, status)
-        );
-        INSERT OR IGNORE INTO tracked_assets_new
-            SELECT asset_id, person_name, status, blur_score, crop_width, crop_height, frigate_score
-            FROM tracked_assets;
-        DROP TABLE tracked_assets;
-        ALTER TABLE tracked_assets_new RENAME TO tracked_assets;
-    """)
+    # Use individual execute() calls inside a transaction — executescript() issues an
+    # implicit COMMIT before running, so a crash between DROP and RENAME would
+    # permanently destroy the table with no rollback path.
+    with conn:
+        conn.execute("""
+            CREATE TABLE tracked_assets_new (
+                asset_id      TEXT NOT NULL,
+                person_name   TEXT,
+                status        TEXT NOT NULL CHECK(status IN ('uploaded', 'rejected')),
+                blur_score    REAL,
+                crop_width    INTEGER,
+                crop_height   INTEGER,
+                frigate_score REAL,
+                PRIMARY KEY (asset_id, person_name, status)
+            )
+        """)
+        conn.execute("""
+            INSERT OR IGNORE INTO tracked_assets_new
+                SELECT asset_id, person_name, status, blur_score, crop_width, crop_height, frigate_score
+                FROM tracked_assets
+        """)
+        conn.execute("DROP TABLE tracked_assets")
+        conn.execute("ALTER TABLE tracked_assets_new RENAME TO tracked_assets")
     logger.info("tracked_assets schema migration complete")
 
 
@@ -453,7 +459,7 @@ def reset_person(person_name: str) -> None:
     # Collect Frigate filenames before deleting
     frigate_filenames = list(get_tracked_frigate_filenames(person_name))
     if frigate_filenames:
-        if not os.environ.get("FRIGATE_URL", "").strip():
+        if not _get_frigate_url():
             logger.info("FRIGATE_URL not set — skipping Frigate file deletion for %s", person_name)
         elif delete_frigate_person_files(person_name, frigate_filenames):
             logger.info("Deleted %s Frigate file(s) for %s", len(frigate_filenames), person_name)