fix: code review corrections — dedup O(N²), truncated rejection check, pool warning, path guard

- _dedup_embeddings: rebuild kept_stack only on keep (was every iteration → O(N²))
- _dedup_embeddings: fix quality_score sort key to use explicit None check (falsy-zero)
- _select_by_embedding: add post-dedup pool < limit guard with warning
- executor: use full resp.text for 'face' keyword check; only truncate display snippet
- _safe_person_dir: avoid false "//" prefix when output_dir resolves to filesystem root

CHANGELOG.md

@@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.4.5] - 2026-06-14
+
+### Fixed
+
+- **Near-duplicate dedup O(N²) allocation**: `np.vstack(kept_normed)` was rebuilt on every loop iteration even for candidates that would be dropped; the stack is now rebuilt only when a new item is kept, reducing memory pressure significantly for large pools.
+- **`quality_score` falsy-zero in dedup sort**: the sort key used `c.get("quality_score") or 0.0`, which treated a legitimate `quality_score=0.0` identically to a missing key. Changed to an explicit `None` check so zero is preserved as-is, and object-mode candidates (which have no `quality_score`) continue to sort stably to the back.
+- **Post-dedup pool not re-checked against limit**: after near-duplicate removal the pool could silently shrink below the requested limit with no warning. A second `len < limit` guard now fires after dedup and emits the same "Only N embeddings" warning that the pre-dedup guard does.
+- **`mark_rejected` could miss plain-text 400 bodies longer than 100 bytes**: `error_detail = resp.text[:100]` was being searched for the keyword `"face"` to gate `mark_rejected()`, so a response body with `"face"` after byte 100 would never mark the asset rejected and it would be retried on every future run. The `"face"` check now uses the full response body; truncation is kept only for the displayed snippet.
+- **`_safe_person_dir` raised ValueError for all person names when `output_dir` resolved to `/`**: `base + os.sep` produced `"//"` when base was `"/"`, and valid paths like `/alice` don't start with `"//"`. Fixed by using `base` directly as the prefix when `base == os.sep`.
+
+## [0.4.4] - 2026-06-14
+
+### Added
+
+- **`RESET_PERSON=*` bulk reset**: resets every tracked person at once (deletes their Frigate training files and clears tracker data). Any other value still resets that specific person by name. If a person is literally named `*` they are reset as part of the bulk operation, and a warning is printed to clarify this.
+- **Near-duplicate removal before diversity selection**: a greedy dedup pass now runs after embedding collection and before clustering. Candidates within 0.20 cosine distance of a higher-quality image are dropped, eliminating burst shots and same-event lookalike photos that produce redundant training images. The best-quality frame from each near-identical group is kept. Dropped count is logged per person.
+
+### Fixed
+
+- **HTTP 500 upload errors no longer show Frigate's misleading "Try restarting Frigate" message**: the response body is now logged at debug level only. HTTP 400 detail (e.g. "No face was detected") is still shown since it is actionable.
+- **`RuntimeWarning: Mean of empty slice`** when a person has only one image after quality filtering: `_compute_adaptive_threshold` now returns the floor value immediately when there are no pairwise distances to sample, and the k-medoids cluster count is floored at 1 to prevent `k=0`.
+- **Path traversal guard on output directory**: person names with `../` sequences or absolute paths (e.g. `/etc`) are now rejected before any filesystem operation, logging an error and skipping the job rather than writing outside the output tree.
+
 ## [0.4.3] - 2026-06-14
 
 ### Added

README.md

@@ -2,6 +2,8 @@
 
 [![Docker](https://github.com/sudolulo/winnow/actions/workflows/docker-publish.yml/badge.svg)](https://github.com/sudolulo/winnow/actions/workflows/docker-publish.yml) [![Test](https://github.com/sudolulo/winnow/actions/workflows/test.yml/badge.svg)](https://github.com/sudolulo/winnow/actions/workflows/test.yml) [![GitHub release](https://img.shields.io/github/v/release/sudolulo/winnow)](https://github.com/sudolulo/winnow/releases/latest) [![License: AGPL v3](https://img.shields.io/badge/License-AGPL_v3-blue.svg)](LICENSE) [![Immich](https://img.shields.io/badge/Immich-v1.106%2B-blueviolet)](https://immich.app) [![Frigate](https://img.shields.io/badge/Frigate-Ready-brightgreen)](https://frigate.video)
 
+> **Note:** winnow's approach to training Frigate face recognition is not an officially documented workflow — results may vary.
+
 > **Early Development — Use With Caution**
 > winnow is functional but still maturing. Features that modify your Frigate training data — quality replacement, stale mapping cleanup — can remove images from your dataset and are not yet battle-tested at scale. Review the logs after each run and keep backups of your Frigate face training directory until you are confident in the results.
 

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "winnow"
-version = "0.4.3"
+version = "0.4.5"
 description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition and object classification."
 license = "AGPL-3.0-or-later"
 requires-python = ">=3.13"

uv.lock

@@ -2348,7 +2348,7 @@ wheels = [
 
 [[package]]
 name = "winnow"
-version = "0.4.2"
+version = "0.4.4"
 source = { editable = "." }
 dependencies = [
     { name = "croniter" },

winnow/cli.py

@@ -156,9 +156,25 @@ def main() -> None:
         rprint(f"Server: [dim]{Config.IMMICH_URL}[/dim]")
         rprint(f"Output: [dim]{Config.OUTPUT_DIR}[/dim]")
 
-        # Handle RESET_PERSON before anything else
+        # Handle RESET_PERSON before anything else.
+        # RESET_PERSON=* resets every tracked person; any other value resets
+        # that specific person by name.
         reset_person_name = os.environ.get("RESET_PERSON", "").strip()
         if reset_person_name:
+            if reset_person_name == "*":
+                names = list(get_person_summary().keys())
+                if "*" in names:
+                    rprint(
+                        "[yellow]Note: a person literally named '*' exists in the tracker "
+                        "and will be reset along with everyone else.[/yellow]"
+                    )
+                if names:
+                    for name in names:
+                        reset_person(name)
+                    rprint(f"[bold yellow]Reset tracking data for all {len(names)} people.[/bold yellow]")
+                else:
+                    rprint("[dim]No tracking data to reset.[/dim]")
+            else:
                 reset_person(reset_person_name)
                 rprint(f"[bold yellow]Reset tracking data for: {reset_person_name}[/bold yellow]")
 

winnow/diversity.py

@@ -281,7 +281,21 @@ def _select_by_embedding(
         logger.warning(f"Only {len(valid_candidates)} valid embeddings. Returning all.")
         return valid_candidates
 
-    # --- Phase 5: Cluster-aware selection ---
+    # --- Phase 5: Near-duplicate removal ---
+    # Burst shots and repeated near-identical photos produce embeddings that are
+    # close but not identical, so FPS doesn't filter them out on its own.
+    # Greedily drop any candidate within DEDUP_THRESHOLD cosine distance of a
+    # higher-quality image already in the kept set.
+    embeddings, valid_candidates, confidence_scores = _dedup_embeddings(
+        embeddings, valid_candidates, confidence_scores
+    )
+
+    # Re-check after dedup: pool may have shrunk below limit
+    if limit != "auto" and len(valid_candidates) < limit:
+        logger.warning(f"Only {len(valid_candidates)} embeddings after near-duplicate removal. Returning all.")
+        return valid_candidates
+
+    # --- Phase 6: Cluster-aware selection ---
     return _cluster_aware_selection(
         embeddings,
         valid_candidates,
@@ -291,6 +305,60 @@ def _select_by_embedding(
     )
 
 
+# =============================================================================
+# Near-Duplicate Removal
+# =============================================================================
+
+_DEDUP_THRESHOLD = 0.20  # cosine distance — burst shots ~0.01-0.05, same-event similar shots ~0.10-0.20
+
+
+def _dedup_embeddings(
+    embeddings: list,
+    candidates: list,
+    confidence_scores: list,
+) -> tuple[list, list, list]:
+    """Greedy near-duplicate removal before clustering.
+
+    Sorts by quality score descending (best first), then for each candidate
+    drops it if any already-kept embedding is within _DEDUP_THRESHOLD cosine
+    distance. This eliminates burst-shot near-duplicates while preserving the
+    highest-quality representative from each near-identical group.
+    """
+    if len(embeddings) < 2:
+        return embeddings, candidates, confidence_scores
+
+    emb_matrix = np.vstack(embeddings)
+    norms = np.linalg.norm(emb_matrix, axis=1, keepdims=True)
+    emb_normed = emb_matrix / np.maximum(norms, 1e-8)
+
+    # Sort by quality descending so the best image in each near-duplicate group wins.
+    # Use explicit None check so a legitimate quality_score=0.0 isn't treated as missing.
+    quality_scores = [qs if (qs := c.get("quality_score")) is not None else 0.0 for c in candidates]
+    order = sorted(range(len(candidates)), key=lambda i: quality_scores[i], reverse=True)
+
+    kept_indices = []
+    kept_stack: np.ndarray | None = None  # rebuilt only when a new item is kept (not every iteration)
+
+    for i in order:
+        if kept_stack is not None:
+            sims = emb_normed[i] @ kept_stack.T
+            if np.any(sims > 1 - _DEDUP_THRESHOLD):
+                continue
+        kept_indices.append(i)
+        row = emb_normed[i : i + 1]
+        kept_stack = row if kept_stack is None else np.vstack([kept_stack, row])
+
+    dropped = len(embeddings) - len(kept_indices)
+    if dropped:
+        logger.info(f"Near-duplicate removal dropped {dropped} images (threshold {_DEDUP_THRESHOLD}).")
+
+    return (
+        [embeddings[i] for i in kept_indices],
+        [candidates[i] for i in kept_indices],
+        [confidence_scores[i] for i in kept_indices],
+    )
+
+
 # =============================================================================
 # K-Medoids (Lightweight Implementation)
 # =============================================================================
@@ -373,6 +441,8 @@ def _compute_adaptive_threshold(emb_normed: np.ndarray, entity_type: str) -> flo
     # Compute pairwise cosine distances for the sample
     pairwise = 1 - sample @ sample.T
     upper_tri = pairwise[np.triu_indices(len(sample), k=1)]
+    if len(upper_tri) == 0:
+        return 0.05
     median_dist = float(np.median(upper_tri))
 
     # Faces: 20% of median (tighter — want fewer, more distinct images)
@@ -421,7 +491,7 @@ def _cluster_aware_selection(
     target = Config.MAX_AUTO_IMAGES if limit == "auto" else limit
 
     # --- Stage 1: K-Medoids clustering ---
-    k = min(max(5, target // 4), n // 3, n)  # e.g., 5-20 clusters
+    k = min(max(5, target // 4), max(1, n // 3), n)  # e.g., 1-20 clusters
     logger.debug(f"Clustering {n} embeddings into {k} groups (K-Medoids)...")
 
     # Compute full cosine distance matrix

winnow/executor.py

@@ -38,6 +38,22 @@ from .upload_tracker import (
 logger = logging.getLogger(__name__)
 
 
+def _safe_person_dir(output_dir: str, person_name: str) -> str:
+    """Return the output subdirectory for a person, raising ValueError on path traversal.
+
+    os.path.join silently discards output_dir when person_name is absolute,
+    and '../..' sequences resolve outside the tree. Both are rejected here.
+    """
+    candidate = os.path.realpath(os.path.join(output_dir, person_name))
+    base = os.path.realpath(output_dir)
+    # Use the base path as its own prefix when it's the filesystem root ("/"),
+    # otherwise append os.sep — avoids the false "//" double-slash when base == "/".
+    base_prefix = base if base == os.sep else base + os.sep
+    if not candidate.startswith(base_prefix) and candidate != base:
+        raise ValueError(f"Person name {person_name!r} escapes output directory — skipping")
+    return candidate
+
+
 def _reconcile_frigate_mappings(
     person_name: str,
     known_files_before: set[str],
@@ -182,7 +198,11 @@ def execute_jobs(jobs: list[dict]) -> None:
             name, mode = person["name"], config.get("mode", "face")
 
             job_task = progress.add_task(f"Processing {name}...", total=len(assets))
-            person_dir = os.path.join(Config.OUTPUT_DIR, name)
+            try:
+                person_dir = _safe_person_dir(Config.OUTPUT_DIR, name)
+            except ValueError as e:
+                logger.error(str(e))
+                continue
             # Face crops are transient (uploaded then discarded); wipe before each run.
             # Object crops are the deliverable; preserve them across runs.
             if mode == "face" and os.path.isdir(person_dir):
@@ -305,7 +325,11 @@ def upload_to_frigate(jobs: list[dict]) -> None:
     object_jobs = [j for j in jobs if j["config"].get("mode") == "object"]
     for job in object_jobs:
         name = job["person"]["name"]
-        person_dir = os.path.join(Config.OUTPUT_DIR, name)
+        try:
+            person_dir = _safe_person_dir(Config.OUTPUT_DIR, name)
+        except ValueError as e:
+            logger.error(str(e))
+            continue
         rprint(f"  [dim]📁 {name} (object): crops saved to {person_dir} — copy to Frigate manually[/dim]")
 
     frigate_url = os.environ.get("FRIGATE_URL", "")
@@ -355,7 +379,11 @@ def upload_to_frigate(jobs: list[dict]) -> None:
             if " " in name:
                 progress.console.print(f"  ℹ️  URL-encoded name for Frigate API: '{name}' → '{encoded_name}'")
 
-            person_dir = os.path.join(Config.OUTPUT_DIR, name)
+            try:
+                person_dir = _safe_person_dir(Config.OUTPUT_DIR, name)
+            except ValueError as e:
+                logger.error(str(e))
+                continue
             if not os.path.isdir(person_dir):
                 progress.console.print(f"  [dim]⏭️  {name}: no output directory, skipping[/dim]")
                 continue
@@ -578,13 +606,16 @@ def upload_to_frigate(jobs: list[dict]) -> None:
                             progress.console.print(
                                 f"    [red]✗ {fname}: HTTP {resp.status_code} (after {max_retries} attempts)[/red]"
                             )
+                            full_body = resp.text
                             try:
-                                error_detail = resp.json().get("message", resp.text[:100])
-                                progress.console.print(f"      [dim]{error_detail}[/dim]")
+                                error_detail = resp.json().get("message", full_body[:100])
                             except Exception:
-                                error_detail = resp.text[:100]
+                                error_detail = full_body[:100]
+                            if resp.status_code == 400:
                                 progress.console.print(f"      [dim]{error_detail}[/dim]")
-                            if resp.status_code == 400 and "face" in error_detail.lower():
+                            else:
+                                logger.debug(f"{fname} HTTP {resp.status_code}: {error_detail}")
+                            if resp.status_code == 400 and "face" in full_body.lower():
                                 asset_id = asset_map.get(fname)
                                 if asset_id:
                                     mark_rejected(asset_id, person_name=name)