Merge pull request #12 from sudolulo/feature/document-limitations

docs+fix: annotate limitations; fix cache invalidation and version checks

winnow/cache.py

@@ -7,37 +7,62 @@ recomputing on reruns. Uses numpy binary format for fast I/O.
 import hashlib
 import logging
 import os
+from pathlib import Path
 
 import numpy as np
 
 logger = logging.getLogger(__name__)
 
-# Model versions — bump these when the upstream model changes
 MODEL_VERSIONS = {
-    "insightface": "buffalo_l_v1",
     "immich": "immich_buffalo_l_v1",
 }
 
 
+def _insightface_model_fingerprint() -> str:
+    """Derive a version string from buffalo_l .onnx file sizes and mtimes.
+
+    Changes automatically when model files are replaced or updated, preventing
+    stale embeddings from a previous model being served from cache.
+    Falls back to a static string before the model is downloaded (first run).
+    """
+    insightface_home = os.environ.get("INSIGHTFACE_HOME", os.path.expanduser("~/.insightface"))
+    model_dir = Path(insightface_home) / "models" / "buffalo_l"
+    if not model_dir.exists():
+        return "buffalo_l_v1"
+    onnx_files = sorted(model_dir.glob("*.onnx"))
+    if not onnx_files:
+        return "buffalo_l_v1"
+    fingerprint = "|".join(
+        f"{f.name}:{f.stat().st_size}:{int(f.stat().st_mtime)}"
+        for f in onnx_files
+    )
+    return hashlib.sha256(fingerprint.encode()).hexdigest()[:12]
+
+
 class EmbeddingCache:
     """Simple disk-based embedding cache.
 
     Embeddings are stored as .npy files in a flat directory,
-    keyed by a hash of (asset_id, model_version).
+    keyed by a hash of (asset_id, model_version). The InsightFace version
+    is derived from buffalo_l model file metadata so the cache auto-invalidates
+    when model files are replaced or updated.
     """
 
     def __init__(self, cache_dir: str = ".if_cache") -> None:
         self.cache_dir = cache_dir
         self._ensured = False
+        self._model_versions = {
+            **MODEL_VERSIONS,
+            "insightface": _insightface_model_fingerprint(),
+        }
 
     def _ensure_dir(self) -> None:
         if not self._ensured:
             os.makedirs(self.cache_dir, exist_ok=True)
             self._ensured = True
 
-    @staticmethod
-    def _key(asset_id: str, model: str) -> str:
-        version = MODEL_VERSIONS.get(model, model)
+    def _key(self, asset_id: str, model: str) -> str:
+        version = self._model_versions.get(model, model)
         raw = f"{asset_id}:{version}"
         return hashlib.sha256(raw.encode()).hexdigest()[:16]
 
@@ -90,4 +115,3 @@ def get_cache(cache_dir: str = ".if_cache") -> EmbeddingCache:
     if _cache is None:
         _cache = EmbeddingCache(cache_dir)
     return _cache
-

winnow/cli.py

@@ -9,7 +9,7 @@ from rich.prompt import Confirm
 
 from .config import Config, ConfigManager
 from .executor import execute_jobs, upload_to_frigate
-from .immich_api import get_people, merge_people
+from .immich_api import get_immich_version, get_people, merge_people
 from .jobs import _show_preview, auto_configure, interactive_configure
 from .log_config import console, setup_logging
 from .upload_tracker import find_by_crop_dimension, get_person_summary, reset_person
@@ -216,6 +216,13 @@ def main() -> None:
                     f" {counts['rejected']} rejected{frigate_part}[/dim]"
                 )
 
+        _immich_version = get_immich_version()
+        if _immich_version is not None and _immich_version < (1, 106, 0):
+            rprint(
+                f"  [yellow]⚠  Immich {'.'.join(str(x) for x in _immich_version)} detected — "
+                "winnow requires v1.106+. Some features may not work.[/yellow]"
+            )
+
         people = get_people()
         if not people:
             rprint("[bold red]Could not fetch people from Immich. Check URL/Key.[/bold red]")

winnow/diversity.py

@@ -203,6 +203,13 @@ def _select_by_embedding(
     # Process in bounded batches so at most _BATCH decoded images live in RAM
     # at once. With 472 candidates each thumbnail is ~3-8 MB decoded; loading
     # all at once easily exhausts a 4 GB container limit on CPU.
+    # LIMITATION — thumbnail-resolution embeddings drive full-res crop selection:
+    # diversity selection runs InsightFace on Immich preview thumbnails (~720p)
+    # to avoid downloading full-res for every candidate, but the training crop
+    # comes from the full-resolution original. Embeddings from thumbnails are
+    # representative in practice, but heavy JPEG compression on a preview could
+    # produce a subtly different embedding than the full-res version. For most
+    # libraries this is negligible; it matters if Immich preview quality is low.
     from concurrent.futures import ThreadPoolExecutor, as_completed
 
     _BATCH = 32

winnow/executor.py

@@ -17,6 +17,7 @@ from .frigate_api import (
     delete_frigate_person_files,
     get_all_frigate_person_files,
     get_frigate_person_files,
+    get_frigate_version,
     recognize_face,
 )
 from .image_processing import process_face_mode
@@ -311,6 +312,18 @@ def upload_to_frigate(jobs: list[dict]) -> None:
         rprint("[yellow]⚠️  FRIGATE_URL not set, skipping upload.[/yellow]")
         return
 
+    _frigate_version = get_frigate_version()
+    if _frigate_version is not None:
+        try:
+            parts = [int(x) for x in _frigate_version.split("-")[0].split(".") if x.isdigit()]
+            if len(parts) >= 2 and (parts[0], parts[1]) < (0, 16):
+                rprint(
+                    f"  [yellow]⚠  Frigate {_frigate_version} detected — "
+                    "face training API requires v0.16+. Uploads may fail.[/yellow]"
+                )
+        except Exception:
+            pass
+
     rprint("\n[bold cyan]📤 Uploading to Frigate[/bold cyan]")
     rprint(f"  Target: [dim]{frigate_url}[/dim]")
 
@@ -380,6 +393,14 @@ def upload_to_frigate(jobs: list[dict]) -> None:
             # manually-added Frigate files don't consume winnow's managed quota.
             # Replacement targets also come exclusively from the tracker, so manually
             # added files are never selected for deletion — only winnow-uploaded ones.
+            # LIMITATION — manual files are invisible to diversity decisions: winnow
+            # can observe their effect on the Frigate score (indirectly, via recognize)
+            # but cannot measure their embedding distribution directly. If a user has
+            # 20 manually-added frontals and winnow has room for 20 more, winnow may
+            # add more frontals because it can't see that frontals are already covered.
+            # 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.
             _snapshot = (
                 all_frigate_files.get(name, []) if all_frigate_files is not None
                 else get_frigate_person_files(name)
@@ -447,6 +468,14 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                 # Frigate rebuilds its model asynchronously after any delete (clear + background
                 # thread), so the first recognize call after a deletion returns None — our code
                 # handles this conservatively by skipping that candidate until the next run.
+                # LIMITATION — async rebuild during multi-replacement runs: each deletion in a
+                # single run triggers a background model rebuild in Frigate. Subsequent recognize
+                # calls in the same run may get None (rebuild in progress), causing later
+                # candidates to fall back to blur-score replacement or be skipped entirely.
+                # The more replacements that happen in one run, the worse the scoring gets.
+                # TODO(frigate-api): if Frigate exposes a model generation counter or a
+                # 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 not at_cap or person_has_fscores:

winnow/frigate_api.py

@@ -8,6 +8,24 @@ import requests
 logger = logging.getLogger(__name__)
 
 
+def get_frigate_version() -> str | None:
+    """Fetch Frigate's version string from GET /api/version.
+
+    Returns the version string (e.g. "0.16.0-beta4") or None if FRIGATE_URL
+    is unset, the endpoint is unreachable, or the response is not parseable.
+    """
+    frigate_url = os.environ.get("FRIGATE_URL", "").rstrip("/")
+    if not frigate_url:
+        return None
+    try:
+        resp = requests.get(f"{frigate_url}/api/version", timeout=5)
+        if resp.ok:
+            return resp.text.strip().strip('"')
+        return None
+    except Exception:
+        return None
+
+
 def _get_faces_data() -> dict | None:
     """Fetch raw GET /api/faces response. Returns None if unavailable."""
     frigate_url = os.environ.get("FRIGATE_URL", "").rstrip("/")
@@ -33,6 +51,10 @@ def get_all_frigate_person_files() -> dict[str, list[str]] | None:
         return None
     # Response: {person_name: [file, ...], "train": [...], ...}
     # "train" is a flat pending list, not a person — skip it.
+    # TODO(frigate-api): "train" is the only known special key as of Frigate v0.16.
+    # If Frigate adds other top-level non-person keys, they'll be silently treated
+    # as person names here. Switch to an allowlist or a typed schema when Frigate
+    # documents its response contract.
     return {
         name: files
         for name, files in data.items()
@@ -75,6 +97,16 @@ def recognize_face(file_path: str) -> tuple[str | None, float] | None:
 
     Returns None if FRIGATE_URL is unset, the API is unreachable, no face is
     detected, or face recognition is not enabled in Frigate.
+
+    LIMITATION — mean embedding comparison: the score reflects similarity to
+    the arithmetic mean of all training embeddings, not to individual ones.
+    A bimodal training set (e.g. frontals + profiles) has a mean that sits
+    between both clusters, making candidates from either cluster look more
+    novel than they are. Winnow could add redundant frontals while the score
+    suggests novelty, because the mean is pulled toward profiles.
+    TODO(frigate-api): if Frigate exposes per-file embeddings via the API,
+    replace mean-comparison with nearest-neighbour distance across individual
+    training embeddings for accurate coverage detection.
     """
     frigate_url = os.environ.get("FRIGATE_URL", "").rstrip("/")
     if not frigate_url:

winnow/immich_api.py

@@ -26,6 +26,25 @@ class FaceData:
     image_height: int
 
 
+def get_immich_version() -> tuple[int, int, int] | None:
+    """Fetch Immich server version from GET /api/server/version.
+
+    Returns (major, minor, patch) or None if unreachable or unparseable.
+    """
+    try:
+        resp = requests.get(
+            f"{Config.IMMICH_URL}/api/server/version",
+            headers=get_headers(),
+            timeout=5,
+        )
+        if resp.ok:
+            data = resp.json()
+            return (int(data["major"]), int(data["minor"]), int(data["patch"]))
+        return None
+    except Exception:
+        return None
+
+
 def get_people() -> list[dict]:
     """Fetch all people from Immich."""
     try: