Commit Graph
77 Commits
Author SHA1 Message Date
flan 0906342edf fix: 10 correctness bugs from full codebase audit
- immich_api: .get("items") or [] handles {"items": null} without crashing len()
- immich_api: catch TypeError alongside ValueError in filter_recent_assets for
  timezone-naive fileCreatedAt comparisons
- scheduler: catch SystemExit in addition to KeyboardInterrupt so cli.main()
  cannot kill the long-running scheduler process
- scheduler: reseed croniter from wall-clock time after each run so overrunning
  jobs don't schedule an immediate back-to-back rerun
- config: reject negative YEARS_FILTER values with a warning, reset to default 10
- frigate_api: return True (not False) for empty filenames list — callers cannot
  distinguish no-op from network failure on False
- jobs: warn on unrecognised STRATEGY value instead of silently falling back
- jobs: casefold ONLY_PEOPLE / SKIP_PEOPLE matching so "john doe" matches "John Doe"
- executor: <= → < so a same-score candidate can fill a freed replacement slot
- embeddings: set _insightface_loaded=True on GPU+CPU double-failure to prevent
  N re-init attempts (one per asset) when InsightFace is broken for a whole run
2026-06-17 19:12:58 +00:00
flan 5e0a871314 fix: 2 low findings from audit — consistent 500 match source, accurate return type
Use full_body for the 500 'could not process' permanent-rejection check,
consistent with the 400 'face' check on the line above. error_detail is
truncated to 100 chars via the fallback path, which could silently miss
the phrase in a long response body.

Remove | None from process_face_mode return type — every code path returns
tuple[int,int] or str; None is unreachable. Update docstring to match.
2026-06-17 17:47:31 +00:00
flan 86caffc8d7 fix: 3 audit findings — truthy skip-reason bug, 500 match consistency, CHANGELOG note
1. if saved: → if isinstance(saved, tuple): so string skip-reasons from
   process_face_mode no longer register as successes and create phantom
   asset_map entries with no JPEG on disk. Dead reason/fallback code in
   the else branch now correctly handles str and None returns.

2. "could not process" permanent-rejection check now uses error_detail
   (json message field, falling back to body[:100]) instead of full_body,
   keeping the match consistent with what is displayed to the user.

3. CHANGELOG [Unreleased] breaking-change note for MAX_AUTO_IMAGES 20→5
   so upgrading users know to set the env var if they want the old cap.
2026-06-17 17:31:20 +00:00
flan b69f776378 fix: surface skip reasons and suppress norm_crop FutureWarning
process_face_mode now returns a descriptive string instead of None for
filtered-out faces ("face too small 45x38px, min 90px", "no face
metadata"), so the executor can print a useful reason rather than the
generic "no usable face data".

Also suppresses the InsightFace norm_crop FutureWarning about deprecated
estimate usage, which was noisy at INFO level on every aligned crop.
2026-06-17 17:18:43 +00:00
flan c4910d82ef fix: treat Frigate 500 'Could not process' as permanent rejection
Frigate returns HTTP 500 with 'Could not process' when its face detector
cannot find or embed a face in the uploaded crop — this will never succeed
on retry. Previously these were silently logged at DEBUG and retried on
every future run.

- Surface 500 error details inline (same display path as 400)
- Mark 500 + 'could not process' as a permanent rejection so the asset
  is skipped on future runs instead of retried indefinitely
2026-06-17 17:16:04 +00:00
flan 3fccf9c8f9 fix: 6 findings from codebase audit round 3
cli.py:
- Filter id-less persons from by_name at construction (root fix for all
  bare-subscript crashes downstream — persons with a name but no id are
  excluded from duplicate detection entirely)
- Belt-and-suspenders on warning-path display: p['id'] → p.get('id')
- Extract survivor_id with .get(); skip group if survivor has no id
- Guard merge_ids: skip API call when list is empty after id filtering
- Walrus operator in merge_ids comprehension: p.get("id") called once
  per item instead of twice

executor.py:
- Add cross-reference comment at success-path reset so the for/else
  rollback pairing is explicit for future maintainers
2026-06-17 02:09:16 +00:00
flan eab3d9fe64 fix: 3 correctness bugs from codebase audit round 2
- executor.py: clear min_quality_score_for_slot alongside effective_count
  restore in for/else block; leaving the stale floor from the deleted
  file's score blocked the next candidate from filling the restored slot
- cli.py: guard merge_ids with p.get('id') is not None, consistent with
  the _smaller_duplicate_ids fix; bare p['id'] raised KeyError on any
  person dict missing the id field in the auto-merge path
- immich_api.py: replace bare data['major'/'minor'/'patch'] subscripts
  with .get() in get_immich_version; KeyError was silently swallowed by
  except Exception, causing version-gated flags to disable without warning
2026-06-17 01:54:13 +00:00
flan 068a8e675f fix: 4 correctness bugs from full-codebase audit
- executor: restore effective_count when replacement upload fails all retries
  (delete succeeded but slot was never filled, leaving cap undercount)
- diversity: skip zero-norm embeddings before dedup/FPS selection
  (InsightFace zeros pass dedup with similarity 0 and score distance 1.0,
  getting selected first as maximally diverse)
- cli: exclude None from skip_ids in _smaller_duplicate_ids
  (p.get('id') without None guard lets None into the set, silently
  dropping every other id-less person from the processed list)
- embeddings: select face nearest crop centre instead of largest by area
  (25% margin can pull a bigger neighbouring face into the crop;
  largest-face selection then embeds the wrong person)
2026-06-17 01:12:28 +00:00
flan 25880ded91 fix: address 2 code review findings (round 9)
- executor: revert person_has_fscores=True back into try/except else
  branch; moving it outside in round 8 was a regression — when the
  tracker write fails on the first-ever upload (no prior frigate_scores
  in tracker), setting the flag True prematurely switches at-cap
  replacement into fscore mode, get_most_redundant_mapped_file returns
  None (no entries), and all replacements are silently skipped;
  the flag must only be set when the score is actually written
- diversity: remove dead face_crop-None guard; any face that passes
  assess_quality (≥90 px MIN_FACE_WIDTH) produces a crop ≥135 px
  (face + 25% margin), which is always above the 30 px crop minimum,
  making the guard unreachable; _crop_face_from_thumbnail also calls
  _get_face_bbox internally, so face_bbox is not None guarantees the
  inner bbox check also passes
2026-06-16 23:40:05 +00:00
flan 461ceb7af4 fix: address 5 code review findings (round 8)
- diversity: fix hard_count regression from round 7 — revert to
  'is not None and < 0.85' so only images that actually receive a
  FPS boost (confirmed low confidence) are counted as hard examples;
  None-confidence images use conf_array=1.0 (no boost) and should
  not appear in the hard-example log count
- diversity: fix _scale_bbox_to_thumbnail to use explicit zero-guard
  for imageWidth/imageHeight (meta_w or 0; scale = img_w/meta_w if
  meta_w else 1.0) — mirrors image_processing.py pattern; prevents
  `or img_w` from silently treating imageWidth=0 as missing and
  returning scale=1.0 without surfacing the zero-metadata case
- embeddings: wrap all three os.close calls in _suppress_output
  finally block with try/except OSError: pass so a failed close
  in one branch cannot abort the outer finally and leak devnull_fd
  or the saved_err/saved_out fds
- upload_tracker: clear corrupt flat-list key (data[flat_key] = [])
  after the isinstance warning instead of leaving the corrupt value
  in place — prevents stale IDs persisting across reset_person calls
  and future load_uploaded_ids() from seeing a non-list value
- executor: move 'if pre_fscore is not None: person_has_fscores = True'
  out of the try/except else branch so it fires even when mark_uploaded
  raises; Frigate scores exist once measured regardless of tracker
  write success, and replacement strategy should reflect that
2026-06-16 23:24:11 +00:00
flan 7282c76b68 fix: address 5 code review findings (round 7)
- diversity: revert conf_array default from 0.5 back to 1.0 (np.ones);
  the 0.5 default caused None-confidence images to receive a 1.7× FPS
  boost and beat high-confidence detections — counter-productive for
  Frigate training data quality
- diversity: fix hard_count to include None-confidence images (count
  images where score is None or < 0.85, not only confirmed < 0.85);
  the previous check systematically undercounted boosted images when the
  Immich faces API omits the score field
- executor: fix garbled comment fragment "Skipped on / skipped when"
  left by a partial edit in round 4; merge into a single coherent sentence
- executor: expand actually_uploaded trade-off comment to document all
  three consequences of a tracker write failure (Frigate duplicate,
  quality-replacement exclusion, cap-slot consumption), not only the
  duplicate risk mentioned previously
- upload_tracker: add person_ids guard to reset_person isinstance check
  so the non-list warning only fires when cleanup would actually have run,
  not on no-op calls where person_ids is empty
2026-06-16 23:09:14 +00:00
flan 692d77ee9f fix: address 4 code review findings (round 6)
- executor: snapshot has_frigate_model = effective_count > 0 before the
  upload loop; use it in the recognize_face gate instead of the live
  effective_count, which is incremented mid-loop and would otherwise
  trigger recognize_face calls against an empty Frigate model on first run
- jobs: restore if already_uploaded > 0 guard before limit = capacity so
  first-run auto-strategy jobs keep limit="auto" and the FPS adaptive
  early-stop can fire instead of always filling MAX_AUTO_IMAGES slots
- cli: retry get_people() once after a post-merge empty response before
  falling back to the pre-merge list; improve warning to name expired API
  key as a possible cause alongside transient network errors
- diversity: hoist hard_weight = np.where(...) above the FPS while loop
  since conf_array is constant; eliminates one O(n) numpy pass per
  selected image
2026-06-16 22:22:51 +00:00
flan 2cb126a589 fix: address 5 code review findings (round 5)
- embeddings: move os.close into try/finally so saved_out/saved_err are
  always closed even when os.dup2 restore raises, preventing fd leak
- cache: replace narrow except tuple with except MemoryError: raise /
  except Exception: return None so struct.error and other np.load failures
  return None without masking OOM
- executor: fix first-run advisory message to check effective_count == 0
  (post-stale-cleanup) instead of pre_run_count; remove now-unused
  pre_run_count variable entirely
- jobs: remove dead "skip" entry from strategy_map (unreachable since the
  early-return at the top of _resolve_strategy fires first)
- upload_tracker: log a warning when reset_person encounters a non-list
  flat_key value instead of silently skipping the cleanup
2026-06-16 22:03:59 +00:00
flan 96099ed6e2 fix: address 8 code review findings (round 4)
- diversity: remove erroneous break outside if-faces in _scale_bbox_to_thumbnail
  (broke people-loop early for first unannotated person, defeating scale fix)
- diversity: increment quality_filtered for face-too-small crop skips so the
  summary log counts them alongside assess_quality failures
- diversity: fix hard-example log count to use original confidence_scores[i]
  instead of synthetic conf_array default (0.5), eliminating false 100%
  hard-example reports for persons with no Immich confidence data
- cache: add EOFError to except tuple in EmbeddingCache.get() so truncated
  .npy files return None instead of crashing the embedding pipeline
- embeddings: wrap each os.dup2 restore in its own try/except OSError in
  _suppress_output finally block so stderr is always restored even if the
  stdout restore raises
- executor: gate recognize_face on effective_count > 0 (post-stale-cleanup)
  instead of pre_run_count > 0 so recognize_face is not called against an
  untrained Frigate model after the user manually deletes all training files
- executor: document actually_uploaded trade-off in comment (appending
  unconditionally on tracker failure risks a Frigate duplicate but prevents
  permanent filename unmapping which breaks quality-replacement scoring)
- jobs: check strategy == "skip" before the has_embedding and custom_limit
  early-returns in _resolve_strategy so STRATEGY=skip is always honoured
2026-06-16 21:43:24 +00:00
flan 4af9da2550 fix: address 10 code review findings (round 3)
- diversity: scale face bbox to thumbnail space before quality check so
  check_face_size uses actual thumbnail pixels, not original-image coords
- diversity: skip asset when face bbox exists but crop guard rejects it,
  preventing InsightFace from picking the wrong person in a group photo
- diversity: add _scale_bbox_to_thumbnail helper (extracted from crop logic)
- diversity: use set for medoid membership test in _kmedoids (O(n) not O(n*k))
- diversity: remove dead np.unique in _select_time_spread (linspace produces
  strictly increasing indices; unique is a no-op and implies wrong semantics)
- embeddings: move os.open/os.dup calls inside try in _suppress_output so
  EMFILE during setup does not leak already-allocated fds
- immich_api: count and log assets with missing/unparseable fileCreatedAt in
  filter_recent_assets instead of silently discarding them
- executor: capture pre_run_count before stale-mapping cleanup so the
  "first run" coaching message doesn't fire after manual file deletion
- cli: use p['id'] (KeyError-safe) instead of p.get('id') in fallback path
  to match all other access sites on the same people list
- cache: narrow except to (OSError, ValueError) in EmbeddingCache.get so
  MemoryError propagates instead of converting OOM to a silent cache miss
2026-06-16 21:17:31 +00:00
flan 8bdce9253a fix: address 10 codebase audit findings — API guards, reconcile, merge fallback, tracker guards
- immich_api: guard resp.json() with isinstance(dict) check in get_people and
  fetch_all_assets so AttributeError doesn't escape on proxy/CDN non-dict responses
- executor: move actually_uploaded.append outside try/else so Frigate filename→asset_id
  mapping is created via reconcile even when the tracker write fails
- cli: fall back to pre-merge people list when re-fetch after merge returns empty
  (transient error) instead of silently dropping all people
- cli: treat ENABLE_FRIGATE_SCORES=false / BLUR_THRESHOLD=0 as not-set in
  the unsupported-vars warning (falsy string check replaces raw truthiness)
- upload_tracker: guard set(data[flat_key]) with isinstance(list) check in
  reset_person so a corrupted non-iterable legacy field doesn't crash mid-reset
- upload_tracker: guard dims[0]/dims[1] in find_by_crop_dimension with a
  length check so a truncated crop_dims entry doesn't raise IndexError
- cache: wrap os.remove() in clear() with try/except OSError to handle
  TOCTOU race with concurrent put() calls
- diversity: default conf_array to 0.5 (was 1.0) for faces with missing
  confidence so they receive a moderate diversity boost instead of being
  treated as high-confidence
- diversity: sort assets in the fast path (len <= limit) so return order is
  consistent with the sorted-by-fileCreatedAt path
2026-06-16 20:51:14 +00:00
flan 34fccf8839 fix: address 10 full-codebase audit findings + lint
Correctness:
- jobs: cap auto-diversity limit for brand-new people (was never capped,
  could exceed MAX_AUTO_IMAGES on first run)
- image_processing: separate None/0 guard for imageWidth/imageHeight so
  missing field is explicit rather than silently aliased to img_w
- upload_tracker (_mark, update_frigate_count): copy-before-mutate so
  exceptions between cache access and _save don't corrupt in-process state
- jobs: reject LIMIT=0 on no-embedding path (was silently empty run)
- jobs: add STRATEGY=skip to strategy_map so env var is honoured
- embeddings: convert to RGB before cvtColor so RGBA/grayscale thumbnails
  don't raise cv2.error and silently drop from diversity selection
- config: use falsy guard for OUTPUT_DIR so blank env var falls through
  to config file value
- reconcile: _ts() returns float("inf") on parse failure so unrecognised
  filenames sort last instead of collapsing to 0.0 and corrupting FIFO mapping
- diversity: remove dead selected_set (never read; -np.inf sentinel already
  prevents re-selection)

Lint (ruff):
- executor: sort upload_tracker import block (I001)
- executor: replace lambda is_better_than with operator.lt/gt (E731 x2)
- executor, upload_tracker: wrap long logger.warning calls (E501 x4)
2026-06-16 18:40:13 +00:00
flan 14f759e960 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.
2026-06-16 18:13:36 +00:00
flan 54b52b0a73 fix: address 3 quality review findings — batch reject tracker, skip flush when clean, hoist frigate url check 2026-06-16 17:55:34 +00:00
flan b622e58f1b fix: address 3 quality review findings — flush_batch finally guard, _laplacian_var helper, has_frigate_scores no-copy 2026-06-16 17:09:00 +00:00
flan f3622b8d41 fix: address 4 quality review findings — flush_batch order, batch finally guard, cache copy, LIMIT=0 fallthrough 2026-06-16 16:59:43 +00:00
flan 817fa17e41 fix: address 3 quality review findings — tracker_ok gate, LIMIT=0 warning, cache write log level 2026-06-16 16:41:32 +00:00
flan a6bae5da05 fix: address 10 audit findings — import bug, fscore stale flag, cache mutation, batch safety, falsy guards 2026-06-16 16:16:28 +00:00
flan 4cdd4657d6 fix: v0.6.2 — structural tracker refactor, batch writes, multi-instance prep
- Drop flat list as primary storage; derive uploaded/rejected IDs from by_person
  (single source of truth). Legacy flat lists in existing files still read for
  backward compat. Removes dual-representation sync hazard.
- Add begin_batch/flush_batch: per-person upload loop now does 1 os.replace
  instead of N (one per mark_uploaded call). Benefit on slow storage.
- reset_all_people(): RESET_PERSON=* is now O(1) disk writes instead of O(P^2).
- blur_score_from_image inlines cv2.Laplacian directly, removing assess_quality
  call overhead and decoupling from the full quality pipeline.
2026-06-16 15:44:41 +00:00
flan dc2efb5ac4 fix: v0.6.1 — tracker integrity, quality replacement correctness, code review fixes
- Catch OSError alongside PIL.UnidentifiedImageError for corrupt thumbnails
- Fix quality replacement mode flip mid-loop (person_has_fscores no longer re-evaluated)
- reset_person rebuilds flat list from remaining entries instead of subtracting
- _save cache updated only after os.replace succeeds (prevents cache/disk split-brain)
- Stale Frigate file cleanup uses remove_frigate_files_batch (N writes → 1)
- _migrate_entry deep-copies nested dicts so .pop() cannot mutate the cache
- find_by_crop_dimension and _pick_mapped_file consistent on duplicate asset→file mapping
- Atomic JSON write (tmp + os.replace) guards against truncated files on crash
- get_person_summary uses _migrate_entry instead of three isinstance guards
- Quality floor check allows None-scored candidates through (don't block freed slots)
- Fix comment-only if body (IndentationError on import) in full-res download path
- Merge duplicate if-stale guard into one block
- _flat_key uses constant equality instead of substring match
- remove_frigate_file returns early when person absent (no ghost entries)
- skip_ids extracted to _smaller_duplicate_ids() helper (was duplicated 3×)
- blur_score_from_image returns None on error instead of 0.0
2026-06-16 15:21:12 +00:00
flan 794dbe2a1d revert: replace SQLite tracker with JSON backend (v0.6.0) (#32)
* revert: replace SQLite tracker with JSON backend (v0.6.0)

The SQLite migration (v0.5.0) spawned 21 bug-fix releases in two days:
data-loss risk in the migration layer, schema PK conflicts on per-person
tracking, tracker isolation races under concurrent runs, and a disk-full
error that triggered duplicate Frigate uploads. The complexity cost
outweighs the benefit.

Restored the pre-SQL JSON tracker (frigate_uploaded_ids.json /
frigate_rejected_ids.json in DATA_DIR). Public API is identical — all
callers in executor.py, jobs.py, cli.py, and reconcile.py work unchanged.
Existing JSON files are read automatically; frigate_tracker.db can be
deleted once verified.

* fix: narrow corrupt-thumbnail exception to UnidentifiedImageError; restore IMMICH_URL empty-string fallback

* docs: rewrite v0.6.0 changelog, strip v0.5.x entries, fix README SQLite references

* chore: remove dead get_frigate_filename_for_asset (orphaned since FRIGATE_SCORE_THRESHOLD removal in v0.4.0)

* fix: sort imports in executor.py (ruff I001)
2026-06-15 11:44:41 -04:00
flan 0602c4ac04 fix: v0.5.21 — diversity cap, fetch rejection, tracker isolation, merge dedup, env parsing
- diversity: cap k-medoids seed count at target so _cluster_aware_selection
  never returns more images than requested (violated MAX_AUTO_IMAGES when
  remaining capacity was 1-4 slots); add early return for limit=0 to
  prevent k-medoids from running with a zero budget; slice return to target
  as a final guard
- executor: mark_rejected() when fetch_full_image returns None so assets
  that can't be fetched (both original and preview) aren't retried every run
- executor: wrap mark_uploaded() in its own try/except so a SQLite disk-full
  error after a successful HTTP 200 doesn't retry the Frigate POST (duplicate
  upload) — the upload succeeded; only the tracker write failed
- cli: apply skip_ids deduplication to the re-fetched people list after a
  partial merge (some groups succeed, some fail) so unmerged duplicates
  don't produce two jobs for the same Frigate folder
- jobs: strip whitespace from SKIP_PEOPLE/ONLY_PEOPLE elements on split
  so "Alice, Bob" (space after comma) correctly matches "Bob"
2026-06-15 03:56:57 +00:00
flan 95fb39ed50 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
2026-06-15 03:26:51 +00:00
flan 728b84dc8c fix: address 10 full-codebase audit findings (v0.5.19)
Correctness:
- fetch_face_data: only fall back to faces[0] when person_id is absent;
  previously a missing person match injected a different person's bbox
- upload_tracker: change PK from (asset_id, status) to
  (asset_id, person_name, status); old PK allowed INSERT OR REPLACE to
  silently overwrite person_name when the same photo appeared in two
  people's jobs, breaking quality-replacement JOINs; auto-migrates DBs
- filter_recent_assets: treat years=0 as "no age filter" instead of
  falling through to Config.YEARS_FILTER via falsy `or`
- _is_module_available: return find_spec(...) is not None; find_spec
  returns None (not raises) for absent top-level modules, so the
  previous code always returned True
- execute_jobs error handler: use asset.get("id", "<unknown>") to avoid
  a secondary KeyError propagating out of execute_jobs on malformed dicts
- upload_to_frigate: also mark_rejected on HTTP 422, not only HTTP 400
  with "face" in body; other permanent errors left assets untracked and
  retried forever
- reconcile_frigate_mappings: sort key lambda f: (_ts(f), f) makes order
  deterministic when timestamps are equal or 0.0; set iteration order is
  hash-randomised, stable sort preserves it

Reuse / cleanup:
- config.py: add _getenv_optional_int delegating to _getenv_num(name, None, int)
- jobs.py: _resolve_strategy uses _getenv_optional_int("LIMIT") instead
  of inline os.environ.get + int() + warning duplicate of _getenv_num
- frigate_api.py: add _get_frigate_url() helper; eliminates 4× copy of
  os.environ.get("FRIGATE_URL", "").rstrip("/")
- quality.py: extract blur_score_from_image(img, max_dim=1440) helper;
  executor.py time-spread blur fallback now uses it instead of inlining
  the resize+RGB+assess_quality sequence, keeping scale logic in one place
2026-06-15 02:56:51 +00:00
flan fe5e1574ac release: v0.5.15 — fix cache regression and structural cleanup
- cache.py: fix np.save extension bug from v0.5.13 — tmp path used
  final+".tmp" (abc.npy.tmp) but np.save auto-appends .npy to paths not
  ending in .npy, writing to abc.npy.tmp.npy instead; os.replace then
  raised FileNotFoundError silently, making every cache write a no-op
  and leaking *.npy.tmp.npy files. Fixed by inserting .tmp before .npy:
  tmp = final[:-4] + ".tmp.npy"

- config.py: remove str(default) round-trip in _getenv_int/_getenv_float
  — use raw = os.getenv(name); return default if raw is None else int(raw)
  so a future float default can't cause a spurious "not a valid integer"
  warning and return the wrong type

- executor.py: consolidate 4 progress.remove_task calls into one
  try/finally around the per-job body; continue inside try/finally
  executes the finally before the next iteration, making the invariant
  structurally enforced rather than relying on discipline across 4 sites
2026-06-15 01:45:43 +00:00
flan 51f7ed3961 release: v0.5.13 — robustness fixes from full-project audit
- executor.py: wrap shutil.rmtree/os.makedirs in try/except OSError so a
  permission failure logs and skips the job rather than aborting the run
- cache.py: write embeddings to a .tmp file and atomically rename into place
  via os.replace so a process kill can't leave a corrupted .npy cache slot
- immich_api.py: guard fileCreatedAt with isinstance(str) check before calling
  .replace() so a non-string timestamp doesn't raise AttributeError and kill
  the entire filter_recent_assets pass
- upload_tracker.py: raise SQLite busy timeout from 5 s to 30 s to handle
  concurrent cron+manual run overlap without dropping upload-tracking records
2026-06-15 01:19:38 +00:00
flan 850ae2f9fc fix: remove progress task on skipped jobs (v0.5.12)
progress.add_task() fires unconditionally at the top of the job loop;
both continue paths (ValueError from _safe_person_dir and the symlink
TOCTOU guard) skipped remove_task(), leaving orphaned 0% rows in the
terminal for the rest of the run.
2026-06-15 01:05:49 +00:00
flan 796aded2da fix: log+skip on symlink TOCTOU in execute_jobs (v0.5.11)
The v0.5.10 compound guard 'isdir and not islink' silently skipped the
rmtree when person_dir was a symlink-to-directory, then let makedirs
follow the symlink — allowing crop writes outside output_dir with no
diagnostic. Replace with an explicit islink pre-check that logs an error
and continues, matching the ValueError path from _safe_person_dir.
2026-06-15 01:02:28 +00:00
flan 480bf80534 fix: reconcile < target severity and rmtree symlink guard (v0.5.10)
- reconcile.py: re-escalate the < target branch from INFO to WARNING and
  add 'permanently unmapped' label. Both post-loop branches produce identical
  permanent mapping loss; v0.5.9 incorrectly treated the timeout case as
  recoverable.

- executor.py: guard shutil.rmtree with 'not os.path.islink(person_dir)'
  so a race-replaced symlink-to-directory is skipped rather than raising
  an unhandled OSError that aborts all remaining jobs. Correct comment:
  rmtree raises OSError, not NotADirectoryError.
2026-06-15 00:58:22 +00:00
flan 2d39291fe7 fix: correct reconcile log severity, docstring gaps, and _entry allocation (v0.5.9)
- reconcile.py: swap log levels — external-upload path (permanent mapping
  loss) escalated to WARNING; timeout path (transient, retries next cycle)
  downgraded to INFO. Also extend the warning message to note the files are
  permanently unmapped.

- immich_api.py: extend fetch_all_assets docstring to document that
  all-garbage page termination (in addition to network errors) makes
  total_raw a lower bound.

- executor.py: add comment above shutil.rmtree noting that POSIX rmtree
  raises NotADirectoryError on a top-level symlink, documenting why the
  removed islink guard is safe to omit.

- upload_tracker.py: replace setdefault with explicit guard in _entry() —
  setdefault evaluates its default-dict argument before checking key
  presence, allocating and discarding a dict on every already-present call.
2026-06-15 00:51:21 +00:00
flan 1556d90bcc fix: correct path traversal docstring, total_raw inflation, and config re-stat (v0.5.8)
- executor.py: fix _safe_person_dir docstring — realpath+startswith is the
  load-bearing traversal guard; islink is a supplementary early-exit for the
  symlink sub-case only. The previous comment "checking after realpath would be
  too late" implied islink was the primary guard, which is backwards.

- immich_api.py: move total_raw accumulation to after the dead-end-page break
  so all-garbage pages don't inflate the count and produce misleading
  "N total, 0 recent" output. Mixed pages (some valid, some non-dict) still
  count page_count so transient schema issues don't shrink MIN_FACE_COUNT below
  threshold. Add warning when a RequestException interrupts pagination mid-way
  so operators know total_raw is a lower bound.

- config.py: eliminate residual TOCTOU — change `if config_file.exists():` to
  `if _data_cfg_exists or config_file.exists():` so _data_cfg is never
  stat'd twice (the v0.5.7 fix cached the first check but not the second).
2026-06-15 00:41:00 +00:00
flan 9685c310af fix: symlink guard placement, fetch_all_assets raw count, config TOCTOU (#31)
executor.py:
- Move islink check into _safe_person_dir on the raw path, before realpath
  resolves it; the previous check at the rmtree site was unreachable dead code
  because realpath already followed any symlink

immich_api.py / jobs.py:
- fetch_all_assets now returns (assets, total_raw) where total_raw is the
  item count seen before non-dict filtering; callers use it for MIN_FACE_COUNT
  guard and display so transient non-dict API items can't incorrectly skip people
- Add WARNING when pagination stops because a page had items but all were non-dict

config.py:
- Cache _data_cfg.exists() in _data_cfg_exists so the dual-config warning
  and config_file selection always read from the same stat() result; previously
  two calls created a TOCTOU window where log and code could disagree

Bump version to 0.5.7
2026-06-14 20:27:16 -04:00
flan 84ebd91929 fix: quality replacement slot floor uses deleted file's score not failed candidate's (#27)
* fix: quality replacement slot floor uses deleted file's score not failed candidate's

When a blur-score replacement deletes a low-quality Frigate file but the
subsequent upload fails, min_quality_score_for_slot was set to candidate_score
(the good file that failed to upload). This filtered out any subsequent
candidate that didn't beat the failed upload, even if it was better than
the file we just deleted — leaving the freed slot unfilled unnecessarily.

The comment on the guard already documented the correct intent: 'require
the next candidate to beat the deleted file's score'. Fix: use target_score
(the deleted file's blur score) as the floor instead of candidate_score.

* chore: bump version to 0.5.4
2026-06-14 19:53:33 -04:00
flan 2e08504682 fix: audit hardening — input validation, error handling, and robustness (#25)
* fix: audit hardening — input validation, error handling, and robustness

- immich_api: guard person["id"] with .get() + early return on missing field
- immich_api: include page number in pagination exception log
- immich_api: validate faces response is a list before indexing
- executor: wrap Image.open() in try/except for non-image HTTP responses
- executor: strip leading 'v' from Frigate version before parsing (v0.16.0 was misread)
- config: wrap FRIGATE_SCORE_CEILING float() parse in try/except with warning
- config: warn when both DATA_DIR and legacy CWD config files exist simultaneously
- scheduler: wrap PID file write in try/except so /tmp failures don't crash startup
- scheduler: clamp sleep to 60s max to bound recovery time after NTP clock jumps
- frigate_api: log unexpected non-list type in get_frigate_person_files at DEBUG

* fix: LIMIT env var crash and symlink guard on person output dir

- jobs: wrap int(LIMIT) parse in try/except — bad value (e.g. "30.5", "all")
  now logs a warning and falls back to the default instead of crashing
- executor: check for symlink before shutil.rmtree on person_dir — prevents
  following a symlink out of OUTPUT_DIR on a shared volume

* chore: bump version to 0.5.3
2026-06-14 19:39:35 -04:00
flan cabdb9c0eb fix: address 9 code review findings
- upload_tracker: partial migration now rolls back atomically on failure;
  JSON renamed only after successful commit so failed runs retry cleanly
- upload_tracker: allowlist score_col in _pick_mapped_file to close
  latent SQL injection surface
- config: move load_dotenv() from module import into _load() so no I/O
  at import time and reset() fully resets env loading
- config: use is None checks for IMMICH_URL/OUTPUT_DIR config-file
  fallback so explicitly empty env vars are not overridden by the file
- executor: skip reconcile when Frigate API is unreachable at upload
  start — tracker baseline is incomplete and would mis-trigger the
  external-upload guard, permanently losing file mappings
- reconcile: change polling break condition from >= to == target so
  transient overshoots don't prematurely exit the loop and trigger
  the external-upload guard
- jobs: apply capacity cap as the selection limit rather than truncating
  post-selection by position, so the diversity algorithm works within
  the right budget from the start
- Dockerfile: explicit gpu branch + exit 1 on unknown VARIANT instead
  of silent fallback
2026-06-14 20:37:51 +00:00
flan e2a1924fb0 refactor: collapse Config proxy, migrate tracker to SQLite, split reconcile module
- Config: remove _ConfigAccessor and ConfigManager; use __getattr__ for lazy
  loading on single _Config class; re-register self as _instance in __getattr__
  so reset() always clears the correct object (item 1)
- upload_tracker: replace hand-rolled JSON store with sqlite3; auto-migrates
  existing JSON on first run; remove dead record_frigate_file function;
  connection re-opens when CACHE_DIR changes for test isolation (items 2, 8)
- diversity: move ThreadPoolExecutor import to module level; inject optional
  fetch_fn parameter for testability (items 3, 6)
- pyproject: consolidate 4 variant files into extras (gpu/rocm/intel/cpu);
  update Dockerfile to use --extra flag; delete variant pyproject/lock files;
  uv.lock needs regen with `uv lock` after this change (item 4)
- jobs: extract _build_job helper to separate business logic from terminal I/O;
  auto_configure delegates dedup/selection to _build_job (item 5)
- logging: convert f-string log calls to % interpolation throughout all winnow/
  modules (item 7)
- reconcile: new module with reconcile_frigate_mappings and
  enrich_asset_with_face_data extracted from executor.py (item 9)
- scheduler: print next scheduled run time after startup and after each run;
  fix f-string logger.error call (item 10)
2026-06-14 19:59:17 +00:00
flan 28424b0f16 fix: implement fixable limitations from annotation pass
cache.py — model fingerprint auto-invalidation:
  Replace hardcoded "buffalo_l_v1" version string with a fingerprint
  derived from buffalo_l .onnx file sizes and mtimes. EmbeddingCache now
  computes this at init time; stale embeddings from replaced or updated
  model files are automatically invalidated. Falls back to the static
  string before the model is downloaded.
  Note: existing caches built against the old key will miss on the first
  run after upgrade and recompute cleanly.

frigate_api.py — Frigate version check:
  Add get_frigate_version() (GET /api/version). Called at the start of
  upload_to_frigate(); warns if below v0.16 where the face training API
  endpoints don't exist.

immich_api.py + cli.py — Immich version check:
  Add get_immich_version() (GET /api/server/version). Called at startup
  before get_people(); warns if below v1.106 where the face data and
  merge APIs winnow depends on aren't guaranteed present.

Remaining TODO(frigate-api) annotations are left in place — they require
Frigate to expose per-file embeddings or a rebuild-complete signal before
they can be addressed.
2026-06-14 18:48:07 +00:00
flan 1d44df6e96 docs: annotate known limitations and Frigate API improvement hooks
Adds inline LIMITATION / TODO(frigate-api) comments at each specific
code site rather than a separate doc that would drift from the code.

frigate_api.py — recognize_face:
  Mean-embedding limitation: score reflects the arithmetic mean of all
  training embeddings. A bimodal set (frontals + profiles) has a mean
  between clusters, making both ends look more novel than they are.
  Fixable if Frigate exposes per-file embeddings for nearest-neighbour
  comparison.

frigate_api.py — get_all_frigate_person_files:
  "train" key exclusion is a hardcoded string. If Frigate adds other
  special top-level keys in /api/faces they'll be silently treated as
  person names. Needs a typed schema when Frigate documents the contract.

executor.py — recognize_face call site:
  Async rebuild: each deletion triggers a background model rebuild in
  Frigate. Subsequent recognize calls in the same run return None
  (rebuild in progress), degrading quality replacement for later
  candidates. Fixable with a rebuild-complete signal from Frigate.

executor.py — effective_count / manual file handling:
  Manually-added files are invisible to diversity decisions. Winnow
  observes their effect only indirectly via the Frigate score, not by
  measuring their embedding distribution. Per-file embeddings from
  Frigate would allow direct diversity measurement against the full set.

executor.py — Frigate version assumption:
  All face training endpoints are v0.16+. No version check at startup;
  failures on older versions are opaque 404s.

cache.py — MODEL_VERSIONS:
  Version string is a hardcoded constant. Manual model file replacement
  (custom weights, InsightFace update) won't invalidate cached embeddings.
  Needs file-checksum-derived versioning or a CLEAR_EMBEDDING_CACHE flag.

diversity.py — thumbnail-resolution embeddings:
  Diversity selection runs InsightFace on preview thumbnails; the actual
  training crop comes from full-resolution originals. Negligible in
  practice but degrades if Immich preview quality is low.
2026-06-14 18:41:37 +00:00
flan 274d50ee99 chore: remove remaining dead-mode references from executor, jobs, compose, Dockerfile
After removing the object pipeline, several dead 'mode' artifacts remained:

- executor.py: unpack `config` from job even though it was no longer read
- jobs.py: set `"mode": "face"` in both configure paths (key never consumed)
- compose.yml: TRAINING_MODE=face env, OBJECT_CLASS comment, HF_HOME, stale MAX_AUTO_IMAGES default note
- Dockerfile: "and object classification" label, /models/huggingface mkdir, HF_HOME ENV
2026-06-14 17:42:27 +00:00
flan 835016e0e3 feat: remove object mode pipeline (YOLO, SigLIP, TRAINING_MODE, OBJECT_CLASS)
winnow is a face recognition training tool. Object mode required manual
file placement with no Frigate API, pulled in torch/torchvision/transformers/
ultralytics (~2 GB), and was architecturally misaligned with the project goal.

Removed:
- process_object_mode (YOLO inference), get_yolo_model
- SigLIP model stack (get_siglip_model, get_object_embedding, batch variant)
- entity_type branching throughout diversity, embeddings, jobs, executor
- TRAINING_MODE and OBJECT_CLASS env vars
- torch, torchvision, transformers, ultralytics dependencies
- pytorch index entries from pyproject.toml
- Object mode from README (Modes section, env var table, How It Works)
2026-06-14 17:29:25 +00:00
flan 105099c819 fix: mark assets rejected when faces API confidence is below threshold
Previously, assets that passed embedding-phase selection but failed the
faces API confidence check in execute_jobs were silently skipped with no
tracker entry. They appeared as valid candidates on every future run,
were re-selected, and re-skipped in an endless cycle. Now they are
marked rejected so they are excluded from future runs. RETRY_REJECTED=true
clears them if Immich later re-processes the image.
2026-06-14 16:46:09 +00:00
flan 0afc9386c6 feat: dynamic Frigate score ceiling; consolidate quality replacement branches
FRIGATE_SCORE_CEILING now defaults to dynamic mode (unset): below-cap
candidates are skipped if their pre-upload Frigate score exceeds the
most-redundant tracked file's score. This catches conditions already
covered by manually-added Frigate images that winnow cannot track —
the embedding-based diversity selection has no visibility into those.
Set FRIGATE_SCORE_CEILING=0 to disable; a positive value (e.g. 0.85)
still acts as a fixed hard ceiling. First-run safety is unchanged
(pre_run_count==0 prevents recognize_face from being called).

The two quality replacement branches (Frigate-score and blur-score)
shared identical structure and are merged into a single code path
parameterised by score source and comparison direction.

Also raises MIN_FACE_COUNT default from 0 to 3 and updates the
config test to match.
2026-06-14 16:38:25 +00:00
flan ec074fd279 fix: remove unused record_frigate_file import (ruff F401) 2026-06-14 04:10:15 +00:00
flan 7dce4a5c71 perf: eliminate O(K²) dedup allocs, vectorize kmedoids cost, batch tracker writes
- _dedup_embeddings: pre-allocated (Q,D) buffer replaces vstack-on-keep,
  dropping O(K²×D) copy overhead down to O(K×D) fill work
- _kmedoids: swap cost sum replaced with numpy fancy-index reduction,
  ~20-50x faster per swap evaluation
- _reconcile_frigate_mappings: O(L) load/save pairs collapsed to one
  batch write via record_frigate_files_batch
2026-06-14 04:03:55 +00:00
flan 3c602e2ef6 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
2026-06-14 03:40:39 +00:00