fix: pagination runaway, double iteration, and reconciliation efficiency (#30)
immich_api.py: - Move empty-page break after non-dict filtering — a page of all-null items no longer loops to MAX_PAGES without terminating - Single-pass partition replaces two inverse isinstance scans per page - Upgrade non-dict item log from DEBUG to WARNING (silent asset loss) reconcile.py: - Check Frigate before the first sleep so fast responses return immediately rather than always paying a 1 s delay - Compute set difference once per poll iteration instead of twice Bump version to 0.5.6
This commit is contained in:
@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [0.5.6] - 2026-06-15
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Pagination runaway on all-non-dict page**: the empty-page break in `fetch_all_assets` now fires after non-dict filtering rather than before, so a page whose items are all non-dict (e.g. all nulls) correctly terminates pagination instead of looping to MAX_PAGES.
|
||||
|
||||
- **Non-dict API items upgraded to warning**: items skipped in a paginated response are now logged at `WARNING` (previously `DEBUG`) so silent asset loss is visible at default log levels.
|
||||
|
||||
- **Single-pass page filtering**: `fetch_all_assets` now partitions valid and invalid items in one loop instead of iterating `page_assets` twice with inverse predicates.
|
||||
|
||||
- **Reconciliation checks Frigate before sleeping**: the poll loop now performs an initial check immediately after upload, then backs off with `_RECONCILE_POLL_DELAYS` only if needed. Previously the loop always slept ≥1 s before any check.
|
||||
|
||||
- **Reconciliation set subtraction computed once**: `current_files - known_files_before` was computed twice per poll iteration (once for the count check, once for the final mapping). It is now computed once and reused.
|
||||
|
||||
## [0.5.5] - 2026-06-15
|
||||
|
||||
### Changed
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "winnow"
|
||||
version = "0.5.5"
|
||||
version = "0.5.6"
|
||||
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"
|
||||
|
||||
@@ -862,7 +862,7 @@ wheels = [
|
||||
|
||||
[[package]]
|
||||
name = "winnow"
|
||||
version = "0.5.5"
|
||||
version = "0.5.6"
|
||||
source = { editable = "." }
|
||||
dependencies = [
|
||||
{ name = "croniter" },
|
||||
|
||||
+15
-6
@@ -115,16 +115,25 @@ def fetch_all_assets(person: dict) -> list[dict]:
|
||||
if isinstance(page_assets, dict):
|
||||
page_assets = page_assets.get("items", [])
|
||||
|
||||
if not page_assets:
|
||||
page_count = len(page_assets) # raw count for termination check before filtering
|
||||
|
||||
# Single pass: partition valid assets from unexpected non-dict items
|
||||
valid_assets, skipped_count = [], 0
|
||||
for item in page_assets:
|
||||
if isinstance(item, dict):
|
||||
valid_assets.append(item)
|
||||
else:
|
||||
skipped_count += 1
|
||||
if skipped_count:
|
||||
logger.warning("%s: skipping %s non-dict item(s) in page %s", name, skipped_count, page)
|
||||
|
||||
if not valid_assets:
|
||||
break
|
||||
|
||||
skipped = [a for a in page_assets if not isinstance(a, dict)]
|
||||
if skipped:
|
||||
logger.debug("%s: skipping %s non-dict item(s) in page %s", name, len(skipped), page)
|
||||
assets.extend(a for a in page_assets if isinstance(a, dict))
|
||||
assets.extend(valid_assets)
|
||||
logger.debug("Fetched page %s, total: %s", page, len(assets))
|
||||
|
||||
if len(page_assets) < page_size or len(assets) >= _MAX_ASSETS_PER_PERSON:
|
||||
if page_count < page_size or len(assets) >= _MAX_ASSETS_PER_PERSON:
|
||||
break
|
||||
|
||||
except (requests.RequestException, ValueError) as e:
|
||||
|
||||
+7
-10
@@ -38,10 +38,12 @@ def reconcile_frigate_mappings(
|
||||
the cost of occasionally missing them when another client is active.
|
||||
"""
|
||||
target = len(uploaded)
|
||||
current_files: set[str] = set()
|
||||
new_files: set[str] = set()
|
||||
|
||||
for delay in _RECONCILE_POLL_DELAYS:
|
||||
time.sleep(delay)
|
||||
# Check before the first sleep so a fast Frigate response returns immediately.
|
||||
for delay in (None, *_RECONCILE_POLL_DELAYS):
|
||||
if delay is not None:
|
||||
time.sleep(delay)
|
||||
fresh = get_frigate_person_files(person_name)
|
||||
if fresh is None:
|
||||
logger.warning(
|
||||
@@ -50,14 +52,9 @@ def reconcile_frigate_mappings(
|
||||
person_name,
|
||||
)
|
||||
return
|
||||
current_files = set(fresh)
|
||||
new_count = len(current_files - known_files_before)
|
||||
if new_count == target:
|
||||
new_files = set(fresh) - known_files_before
|
||||
if len(new_files) >= target:
|
||||
break
|
||||
if new_count > target:
|
||||
break # external upload already visible — no point polling further
|
||||
|
||||
new_files = current_files - known_files_before
|
||||
|
||||
if len(new_files) == target:
|
||||
def _ts(fname: str) -> float:
|
||||
|
||||
Reference in New Issue
Block a user