Auto-import: bounded ThreadPoolExecutor + per-candidate UI state isolation

# Concurrency model

Pre-refactor concurrency was emergent + unbounded:

- The worker's `_run` thread called `_scan_cycle` every 60s,
  processing candidates synchronously in a for-loop.
- The `/api/auto-import/scan-now` endpoint spawned a fresh
  `threading.Thread(target=_scan_cycle)` per click — extra parallel
  scan cycles on top of the timer.
- Multiple "Scan Now" clicks during in-flight processing → multiple
  threads racing on `_processing_paths` / `_folder_snapshots` state,
  no upper bound on concurrent scanners.
- `stop()` didn't wait for in-flight processing — could leave file
  moves / tag writes / DB inserts mid-flight.

Refactor to the pattern Cin uses elsewhere (`missing_download_executor`,
`sync_executor`, `import_singles_executor` all use
`ThreadPoolExecutor(max_workers=3, thread_name_prefix=...)`):

- **One scan thread** — both timer + manual triggers go through
  `trigger_scan()`, gated by a non-blocking `_scan_lock`. Duplicate
  triggers no-op instead of stacking parallel scanners.
- **Bounded executor** — `ThreadPoolExecutor` (default 3 workers,
  configurable via `auto_import.max_workers`) runs per-candidate
  work. Each candidate runs to completion in its own pool thread;
  up to N candidates run in parallel.
- `_scan_and_submit()` is fast — just enumeration + executor submit,
  returns immediately, doesn't block on per-candidate work.
- `_process_one_candidate(candidate)` holds the per-candidate logic
  identical to the old for-loop body, lifted into a method so the
  pool can run multiple instances concurrently.
- `_submitted_hashes` set + lock dedupes candidates across the
  timer + manual triggers so a candidate already queued / running
  doesn't get re-submitted.
- `stop()` calls `executor.shutdown(wait=True)` — clean shutdown,
  no orphaned file ops.

# Per-candidate UI state isolation

The executor refactor opened two concurrency holes that the old
sequential model masked. Both fixed in this commit:

1. **Scalar UI fields stomped across pool workers.** Pre-refactor
   `_current_folder` / `_current_status` / `_current_track_*` were
   safe under the sequential model — only one candidate processed
   at a time, so the fields tracked the in-flight one. With three
   pool workers writing the same fields, the polling UI saw garbage
   like "Processing AlbumA, track 7/14: SongFromAlbumB".
   Replaced with `_active_imports: Dict[hash, _ActiveImport]` keyed
   on folder_hash, gated by `_active_lock`. Each pool worker owns
   its own entry. Helpers `_register_active` / `_update_active` /
   `_unregister_active` / `_snapshot_active` are the only API.

2. **Stats counters not thread-safe.** `self._stats[k] += 1` is
   read-modify-write — under load, parallel pool workers drop
   increments. New `_stats_lock` + `_bump_stat()` helper wraps every
   mutation. `get_status()` reads under the same lock and returns
   a copy.

# Endpoint change

`/api/auto-import/scan-now` no longer spawns its own scan thread —
calls `auto_import_worker.trigger_scan()` (which routes through the
shared lock + executor). Multiple clicks while a scan is in flight
no-op deterministically. Endpoint still wraps the call in a daemon
thread so the HTTP response returns immediately even if the staging
walk is slow.

# Backward compat

The scalar `_current_folder` / `_current_status` / `_current_track_*`
fields are preserved as **read-only properties** that resolve to the
FIRST active import. The existing `get_status()` payload still
includes those fields populated from the first entry — single-import
UIs (and the test fixture) keep working unchanged. New
`active_imports` array exposes the full multi-candidate state for
parallel-aware UIs.

# Behavior preserved

- Per-candidate identify / match / process logic byte-identical
- Live-progress state preserved (per candidate now)
- Stability gate / already-processed dedup preserved
- `_record_in_progress` / `_finalize_result` UI rows preserved
- Tag-based loose-file grouping unchanged

# Behavior changes

- Multiple albums process IN PARALLEL up to `max_workers`
- "Scan Now" while scan in progress no-ops (was: spawned another)
- `stop()` waits for in-flight pool work via `shutdown(wait=True)`
- Auto-import card now lists each in-flight album (one line per
  active import) instead of a single shared progress line

# UI

`webui/static/stats-automations.js`:
- Progress widget reads `active_imports` array, renders one line
  per in-flight album with per-candidate status / track index
- Falls back to the legacy summary line when payload doesn't
  carry `active_imports` (older backend)
- Per-row "live processing" lookup now matches by `folder_hash`
  through the array instead of by `folder_name` against scalars

# Tests added (`tests/imports/test_auto_import_executor.py`)

- Pool config: default max_workers=3, configurable via constructor
  + via `auto_import.max_workers` config, floors at 1
- Scan lock: 5 concurrent `trigger_scan()` calls run only 1 scan
  while lock held; releases properly so subsequent triggers run
- Executor dispatch: 5 candidates → 5 process calls via the pool
- Bounded parallelism: max_workers=3 caps at 3 concurrent;
  max_workers=2 caps at 2
- Cross-trigger dedup: candidate submitted in scan A doesn't get
  re-submitted by scan B while still in-flight
- Graceful shutdown: `stop()` blocks until in-flight pool work
  finishes
- Per-candidate state isolation: 2 parallel workers updating their
  own candidate state don't interfere — each candidate's
  track_index / track_name / folder_name reads back exactly as
  written for that hash
- `get_status()` returns coherent `active_imports` array with
  one entry per in-flight candidate; aggregate top-level
  `current_status` is 'processing' when any entry is processing
- Unregister removes only that candidate, others stay visible
- Stats counter thread-safety: 1000 parallel bumps land at 1000
  (the read-modify-write race regresses without the lock)
- `get_status()` stats snapshot is a copy, not a live reference

# Verification

- 17 new tests pass (executor + state isolation)
- 2347 full suite passes (1 pre-existing flaky test —
  `test_watchdog_warns_about_stuck_workers` — passes in isolation,
  unrelated)
- Ruff clean
This commit is contained in:
Broque Thomas 2026-05-09 17:45:42 -07:00
parent e11786ee40
commit 8a6ee7a2c7
5 changed files with 1004 additions and 183 deletions

View file

@ -15,6 +15,7 @@ import os
import re import re
import threading import threading
import time import time
from concurrent.futures import ThreadPoolExecutor
from dataclasses import dataclass, field from dataclasses import dataclass, field
from datetime import datetime from datetime import datetime
from difflib import SequenceMatcher from difflib import SequenceMatcher
@ -43,6 +44,29 @@ class FolderCandidate:
is_staging_root: bool = False is_staging_root: bool = False
@dataclass
class _ActiveImport:
"""Per-candidate UI state for an in-flight import.
Multiple instances can exist simultaneously when the executor pool
runs candidates in parallel. Each is keyed on `folder_hash` in the
worker's `_active_imports` dict; mutations are gated by
`_active_lock` so the polling UI sees a coherent snapshot.
Pre-refactor the worker had scalar `_current_folder` /
`_current_status` / `_current_track_*` fields stomped by every pool
worker three concurrent imports would interleave each other's
folder name + track index in the UI. This dataclass + the dict
keyed on folder_hash makes per-candidate state isolated.
"""
folder_hash: str
folder_name: str
status: str = 'queued' # 'queued' | 'identifying' | 'matching' | 'processing'
track_index: int = 0
track_total: int = 0
track_name: str = ''
def _compute_folder_hash(audio_files: List[str]) -> str: def _compute_folder_hash(audio_files: List[str]) -> str:
"""Deterministic hash of folder contents for change detection.""" """Deterministic hash of folder contents for change detection."""
items = [] items = []
@ -160,13 +184,36 @@ def _quality_rank(ext: str) -> int:
class AutoImportWorker: class AutoImportWorker:
"""Background worker that watches the staging folder and auto-imports music.""" """Background worker that watches the staging folder and auto-imports music.
Concurrency model:
- **One scan thread** (the `_run` timer loop) enumerates the staging
folder periodically. Manual "Scan Now" requests share the same
scan via `trigger_scan()` non-blocking lock means duplicate
requests no-op instead of stacking up parallel scanners.
- **Bounded process pool** (`ThreadPoolExecutor`, default 3 workers)
handles per-candidate work: identification, matching, file move,
tagging, DB write. Each candidate runs to completion in its own
pool thread; multiple candidates run in parallel up to the pool
size.
- The scan thread is FAST (just enumeration + submit), the pool
threads are SLOW (per-candidate work).
Pre-refactor, the manual-scan endpoint spawned a fresh
`threading.Thread(target=_scan_cycle)` per click emergent
parallelism with no upper bound, no shared queue, no graceful
shutdown. Fixed by routing both the timer + the manual button
through `trigger_scan()` and submitting per-candidate work to a
shared executor.
"""
def __init__(self, database, staging_path: str = './Staging', def __init__(self, database, staging_path: str = './Staging',
transfer_path: str = './Transfer', transfer_path: str = './Transfer',
process_callback: Optional[Callable] = None, process_callback: Optional[Callable] = None,
config_manager: Any = None, config_manager: Any = None,
automation_engine: Any = None): automation_engine: Any = None,
max_workers: int = 3):
self.database = database self.database = database
self.staging_path = staging_path self.staging_path = staging_path
self.transfer_path = transfer_path self.transfer_path = transfer_path
@ -174,43 +221,189 @@ class AutoImportWorker:
self._config_manager = config_manager self._config_manager = config_manager
self._automation_engine = automation_engine self._automation_engine = automation_engine
# Pool size — defaults to 3 to match the existing pool patterns
# (`missing_download_executor`, `sync_executor`,
# `import_singles_executor`). Configurable via the
# `auto_import.max_workers` config key on init; not hot-
# reloadable (the executor is created once and lives for the
# worker's lifetime).
if config_manager:
max_workers = config_manager.get('auto_import.max_workers', max_workers)
self._max_workers = max(1, int(max_workers))
self.running = False self.running = False
self.paused = False self.paused = False
self.should_stop = False self.should_stop = False
self._thread = None self._thread = None
self._stop_event = threading.Event() self._stop_event = threading.Event()
# Bounded executor for per-candidate processing work. Created
# in `start()` so a stopped+restarted worker gets a fresh pool.
self._executor: Optional[ThreadPoolExecutor] = None
# Non-blocking lock that gates concurrent scans. Both the timer
# loop and the manual "Scan Now" endpoint route through
# `trigger_scan()`; a `try-acquire` here means whichever caller
# gets there first runs the scan and the rest no-op.
self._scan_lock = threading.Lock()
# State # State
self._folder_snapshots: Dict[str, float] = {} # path -> mtime_sum self._folder_snapshots: Dict[str, float] = {} # path -> mtime_sum
# Candidates currently being processed (skip on rescan). Keyed # Candidates currently submitted to the pool OR running in a
# on folder_hash, NOT path — multiple candidates can share a # pool worker. Keyed on folder_hash, NOT path — multiple
# path (each loose-file group at staging root has the same # candidates can share a path (each loose-file group at staging
# parent directory but a distinct hash from its own audio # root has the same parent directory but a distinct hash from
# files). Path-keyed dedup would treat siblings as duplicates # its own audio files). Path-keyed dedup would treat siblings
# and silently skip all but the first. # as duplicates and silently skip all but the first.
self._processing_hashes: set = set() # Rebranded from `_processing_hashes` to `_submitted_hashes`
self._current_folder = '' # because submission to the pool happens immediately (queued
self._current_status = 'idle' # 'idle' | 'scanning' | 'processing' # OR running) — both states need to gate next-scan submissions.
# Live per-track progress so the UI can show "Processing Speak Now self._submitted_hashes: set = set()
# (3/14: Mine)" while a multi-track album is being post-processed. self._submitted_lock = threading.Lock()
# Without this, auto-import goes silent for the entire processing
# window (which can be 5+ minutes for a full album) since # Per-candidate UI state, keyed on folder_hash. Multiple pool
# ``_record_result`` only fires after every track is done. # workers populate this dict simultaneously; `_active_lock`
self._current_track_index = 0 # gates every read/write so the polling UI sees a coherent
self._current_track_total = 0 # snapshot. Replaces the scalar `_current_folder` /
self._current_track_name = '' # `_current_status` / `_current_track_*` fields — those were
# safe under the old sequential model but stomped each other
# under parallel executor workers.
self._active_imports: Dict[str, _ActiveImport] = {}
self._active_lock = threading.Lock()
# Whether a scan-cycle (enumeration phase) is currently
# running. Distinct from per-candidate processing — the scan
# is fast (seconds) and runs at most once at a time
# (gated by `_scan_lock`). Per-candidate work runs concurrently
# in the pool, tracked in `_active_imports`.
self._scan_in_progress = False
# `_stats[x] += 1` from multiple pool threads is read-modify-
# write — under load the counters drift. `_stats_lock` gates
# every mutation via `_bump_stat`.
self._stats = {'scanned': 0, 'auto_processed': 0, 'pending_review': 0, 'failed': 0} self._stats = {'scanned': 0, 'auto_processed': 0, 'pending_review': 0, 'failed': 0}
self._stats_lock = threading.Lock()
self._last_scan_time = None self._last_scan_time = None
# ── Per-candidate UI state helpers ──
def _register_active(self, candidate: 'FolderCandidate', status: str = 'queued') -> None:
"""Insert/refresh the active-import entry for a candidate."""
with self._active_lock:
entry = self._active_imports.get(candidate.folder_hash)
if entry is None:
entry = _ActiveImport(
folder_hash=candidate.folder_hash,
folder_name=candidate.name,
status=status,
)
self._active_imports[candidate.folder_hash] = entry
else:
# Refresh in case the candidate name changed across scans
entry.folder_name = candidate.name
entry.status = status
def _update_active(self, folder_hash: str, **fields: Any) -> None:
"""Mutate fields on an active-import entry. No-op if the entry
isn't registered (e.g. test calling helpers directly without
going through `_register_active`)."""
with self._active_lock:
entry = self._active_imports.get(folder_hash)
if entry is None:
return
for key, value in fields.items():
if hasattr(entry, key):
setattr(entry, key, value)
def _unregister_active(self, folder_hash: str) -> None:
with self._active_lock:
self._active_imports.pop(folder_hash, None)
def _snapshot_active(self) -> List[Dict[str, Any]]:
"""Coherent list snapshot for the UI poller. Order is insertion
order so the legacy single-import fields (which read the first
entry) are stable for any given UI poll cycle."""
with self._active_lock:
return [
{
'folder_hash': e.folder_hash,
'folder_name': e.folder_name,
'status': e.status,
'track_index': e.track_index,
'track_total': e.track_total,
'track_name': e.track_name,
}
for e in self._active_imports.values()
]
def _bump_stat(self, key: str) -> None:
"""Thread-safe increment of `_stats[key]`. Pool workers call
this from multiple threads; raw `self._stats[k] += 1` is read-
modify-write and drops counts under load."""
with self._stats_lock:
self._stats[key] = self._stats.get(key, 0) + 1
# Read-only back-compat properties — the test fixture (and the
# polling UI's legacy fields) read these. Resolve to the FIRST
# active import so the existing single-track-progress UI keeps
# working when only one candidate is in flight (the common case).
# When N candidates run in parallel the UI should iterate
# `active_imports` from `get_status()` instead.
@property
def _current_folder(self) -> str:
with self._active_lock:
if not self._active_imports:
return ''
return next(iter(self._active_imports.values())).folder_name
@property
def _current_status(self) -> str:
with self._active_lock:
for e in self._active_imports.values():
if e.status == 'processing':
return 'processing'
if self._active_imports:
# An active import that hasn't reached 'processing' yet
# is still in identification/matching — keep showing
# 'scanning' for the legacy UI (no separate state).
return 'scanning'
return 'scanning' if self._scan_in_progress else 'idle'
@property
def _current_track_index(self) -> int:
with self._active_lock:
if not self._active_imports:
return 0
return next(iter(self._active_imports.values())).track_index
@property
def _current_track_total(self) -> int:
with self._active_lock:
if not self._active_imports:
return 0
return next(iter(self._active_imports.values())).track_total
@property
def _current_track_name(self) -> str:
with self._active_lock:
if not self._active_imports:
return ''
return next(iter(self._active_imports.values())).track_name
def start(self): def start(self):
if self.running: if self.running:
return return
self.should_stop = False self.should_stop = False
self._stop_event.clear() self._stop_event.clear()
self.running = True self.running = True
# Fresh pool per start so a stop+start cycle gets a clean
# executor (the previous one is shut down in `stop()`).
self._executor = ThreadPoolExecutor(
max_workers=self._max_workers,
thread_name_prefix='AutoImport',
)
self._thread = threading.Thread(target=self._run, daemon=True, name='AutoImportWorker') self._thread = threading.Thread(target=self._run, daemon=True, name='AutoImportWorker')
self._thread.start() self._thread.start()
logger.info("Auto-import worker started") logger.info(f"Auto-import worker started (max_workers={self._max_workers})")
def stop(self): def stop(self):
self.should_stop = True self.should_stop = True
@ -218,6 +411,13 @@ class AutoImportWorker:
self.running = False self.running = False
if self._thread and self._thread.is_alive(): if self._thread and self._thread.is_alive():
self._thread.join(timeout=5) self._thread.join(timeout=5)
# Wait for in-flight pool work to finish before reporting
# stopped. Without `wait=True` we'd return while file moves /
# tag writes / DB inserts are still mid-flight, which can
# corrupt state on shutdown.
if self._executor is not None:
self._executor.shutdown(wait=True)
self._executor = None
logger.info("Auto-import worker stopped") logger.info("Auto-import worker stopped")
def pause(self): def pause(self):
@ -229,15 +429,32 @@ class AutoImportWorker:
logger.info("Auto-import worker resumed") logger.info("Auto-import worker resumed")
def get_status(self) -> dict: def get_status(self) -> dict:
active = self._snapshot_active()
# Aggregate top-level status: 'processing' if any active import
# is in the per-track loop, else 'scanning' if a scan or any
# earlier-phase import is in flight, else 'idle'.
if any(a['status'] == 'processing' for a in active):
current_status = 'processing'
elif active or self._scan_in_progress:
current_status = 'scanning'
else:
current_status = 'idle'
# Legacy single-import scalars — pulled from the first active
# entry so the existing UI keeps rendering one folder at a
# time. Multi-import-aware UIs should read `active_imports`.
first = active[0] if active else None
with self._stats_lock:
stats_snapshot = self._stats.copy()
return { return {
'running': self.running, 'running': self.running,
'paused': self.paused, 'paused': self.paused,
'current_folder': self._current_folder, 'current_status': current_status,
'current_status': self._current_status, 'current_folder': first['folder_name'] if first else '',
'current_track_index': self._current_track_index, 'current_track_index': first['track_index'] if first else 0,
'current_track_total': self._current_track_total, 'current_track_total': first['track_total'] if first else 0,
'current_track_name': self._current_track_name, 'current_track_name': first['track_name'] if first else '',
'stats': self._stats.copy(), 'active_imports': active,
'stats': stats_snapshot,
'last_scan_time': self._last_scan_time, 'last_scan_time': self._last_scan_time,
} }
@ -246,7 +463,7 @@ class AutoImportWorker:
return self._stop_event.wait(seconds) return self._stop_event.wait(seconds)
def _run(self): def _run(self):
"""Main worker loop.""" """Main worker loop — calls `trigger_scan()` periodically."""
interval = 60 interval = 60
if self._config_manager: if self._config_manager:
interval = self._config_manager.get('auto_import.scan_interval', 60) interval = self._config_manager.get('auto_import.scan_interval', 60)
@ -262,32 +479,114 @@ class AutoImportWorker:
enabled = self._config_manager.get('auto_import.enabled', False) enabled = self._config_manager.get('auto_import.enabled', False)
if enabled: if enabled:
try: self.trigger_scan()
self._current_status = 'scanning'
self._scan_cycle()
self._last_scan_time = datetime.now().isoformat()
except Exception as e:
logger.error(f"Auto-import scan cycle error: {e}")
finally:
self._current_status = 'idle'
self._current_folder = ''
if self._interruptible_sleep(interval): if self._interruptible_sleep(interval):
break break
def _scan_cycle(self): def trigger_scan(self):
"""One full scan of the staging folder.""" """Run one scan cycle — single canonical entry point for both
the timer loop AND the manual "Scan Now" endpoint.
Non-blocking: if a scan is already running, returns immediately
without spawning a duplicate. The in-flight scan will pick up
any new files anyway, and stacking parallel scanners caused
unbounded thread growth pre-refactor (each "Scan Now" click
spawned a fresh `_scan_cycle` thread).
Per-candidate processing happens on the bounded executor pool
this method just enumerates + submits, so it returns fast.
"""
if not self._scan_lock.acquire(blocking=False):
logger.debug("[Auto-Import] Scan already running, skipping duplicate trigger")
return
try:
self._scan_in_progress = True
self._scan_and_submit()
self._last_scan_time = datetime.now().isoformat()
except Exception as e:
logger.error(f"Auto-import scan cycle error: {e}")
finally:
self._scan_in_progress = False
self._scan_lock.release()
def _scan_and_submit(self):
"""Enumerate staging candidates + submit each to the executor.
Fast does NOT block on per-candidate processing. The pool
runs `_process_one_candidate` in parallel up to `max_workers`.
"""
staging = self._resolve_staging_path() staging = self._resolve_staging_path()
if not staging or not os.path.isdir(staging): if not staging or not os.path.isdir(staging):
logger.warning(f"[Auto-Import] Staging path not found or invalid: {self.staging_path}") logger.warning(f"[Auto-Import] Staging path not found or invalid: {self.staging_path}")
return return
# Find folder candidates
candidates = self._enumerate_folders(staging) candidates = self._enumerate_folders(staging)
logger.info(f"[Auto-Import] Scan cycle: {len(candidates)} candidates in {staging}") logger.info(f"[Auto-Import] Scan cycle: {len(candidates)} candidates in {staging}")
if not candidates: if not candidates:
return return
if self._executor is None:
logger.warning("[Auto-Import] Executor not initialized — skipping scan")
return
for candidate in candidates:
if self.should_stop or self.paused:
break
# Skip if already processed (DB-level dedup)
if self._is_already_processed(candidate.folder_hash):
continue
# Skip if already submitted to / running in the pool. This
# de-dupes across the timer loop + manual scan triggers
# (both share the `_submitted_hashes` set).
with self._submitted_lock:
if candidate.folder_hash in self._submitted_hashes:
logger.debug(
f"[Auto-Import] Skipping {candidate.name}"
f"already queued in pool"
)
continue
# Stability gate (files not changing). Done OUTSIDE the
# submitted-hashes critical section so a slow stat() call
# doesn't hold the lock across other candidates.
if not self._is_folder_stable(candidate):
continue
with self._submitted_lock:
# Re-check inside the lock — another scanner could have
# claimed this candidate between the first check + here.
if candidate.folder_hash in self._submitted_hashes:
continue
self._submitted_hashes.add(candidate.folder_hash)
try:
self._executor.submit(self._process_one_candidate, candidate)
except RuntimeError as exc:
# Executor was shut down while we were submitting —
# release our claim so a future scan can retry.
logger.debug("[Auto-Import] Executor rejected submit: %s", exc)
with self._submitted_lock:
self._submitted_hashes.discard(candidate.folder_hash)
def _process_one_candidate(self, candidate: 'FolderCandidate'):
"""Per-candidate processing — runs in a pool worker thread.
Identical logic to the old `_scan_cycle` for-loop body, just
moved into a method so the executor can run multiple
candidates in parallel.
Each pool worker registers its candidate in `_active_imports`
on entry + unregisters on exit. UI status fields are scoped
per-candidate so concurrent workers don't stomp each other.
"""
self._bump_stat('scanned')
self._register_active(candidate, status='identifying')
logger.info(f"[Auto-Import] Processing folder: {candidate.name} ({len(candidate.audio_files)} files)")
threshold = 0.9 threshold = 0.9
if self._config_manager: if self._config_manager:
threshold = self._config_manager.get('auto_import.confidence_threshold', 0.9) threshold = self._config_manager.get('auto_import.confidence_threshold', 0.9)
@ -296,134 +595,96 @@ class AutoImportWorker:
if self._config_manager: if self._config_manager:
auto_process = self._config_manager.get('auto_import.auto_process', True) auto_process = self._config_manager.get('auto_import.auto_process', True)
for candidate in candidates: try:
if self.should_stop or self.paused: # Phase 3: Identify
break identification = self._identify_folder(candidate)
if not identification:
self._record_result(candidate, 'needs_identification', 0.0,
error_message='Could not identify album from tags, folder name, or fingerprint')
self._bump_stat('failed')
return
self._current_folder = candidate.name # Phase 4: Match tracks
self._update_active(candidate.folder_hash, status='matching')
match_result = self._match_tracks(candidate, identification)
if not match_result:
self._record_result(candidate, 'needs_identification', 0.0,
album_id=identification.get('album_id'),
album_name=identification.get('album_name'),
artist_name=identification.get('artist_name'),
image_url=identification.get('image_url'),
error_message='Could not match tracks to album tracklist')
self._bump_stat('failed')
return
# Skip candidates currently being processed by a previous confidence = match_result['confidence']
# scan cycle. Keyed on folder_hash because multiple status = 'matched'
# candidates can share a path (loose-file groups at the
# same directory level each get their own candidate but
# share the parent directory).
if candidate.folder_hash in self._processing_hashes:
logger.debug(f"[Auto-Import] Skipping {candidate.name} — still processing from previous cycle")
continue
# Check if already processed # Check if individual track matches are strong even if overall confidence
if self._is_already_processed(candidate.folder_hash): # is low (e.g. only 2 of 18 album tracks present → low coverage kills
continue # overall score, but the 2 tracks match perfectly and should still import)
high_conf_matches = [m for m in match_result.get('matches', []) if m['confidence'] >= 0.8]
has_strong_individual_matches = len(high_conf_matches) > 0
# Check stability (files not changing) if (confidence >= threshold or has_strong_individual_matches) and auto_process:
if not self._is_folder_stable(candidate): # Phase 5: Auto-process — insert an in-progress row
continue # so the UI sees the import the moment it starts,
# then update it with the final status when done.
effective_conf = max(confidence, min(m['confidence'] for m in high_conf_matches) if high_conf_matches else 0)
logger.info(f"[Auto-Import] Processing {candidate.name}"
f"overall: {confidence:.0%}, {len(high_conf_matches)} strong matches, "
f"{match_result.get('matched_count', 0)}/{match_result.get('total_tracks', '?')} tracks")
self._stats['scanned'] += 1 in_progress_row_id = self._record_in_progress(
logger.info(f"[Auto-Import] Processing folder: {candidate.name} ({len(candidate.audio_files)} files)") candidate, identification, match_result,
)
self._update_active(candidate.folder_hash, status='processing')
# Mark as in-progress so next scan cycle skips this candidate success = self._process_matches(candidate, identification, match_result)
self._processing_hashes.add(candidate.folder_hash) status = 'completed' if success else 'failed'
try: confidence = max(confidence, effective_conf)
# Phase 3: Identify if success:
identification = self._identify_folder(candidate) self._bump_stat('auto_processed')
if not identification:
self._record_result(candidate, 'needs_identification', 0.0,
error_message='Could not identify album from tags, folder name, or fingerprint')
self._stats['failed'] += 1
continue
# Phase 4: Match tracks
match_result = self._match_tracks(candidate, identification)
if not match_result:
self._record_result(candidate, 'needs_identification', 0.0,
album_id=identification.get('album_id'),
album_name=identification.get('album_name'),
artist_name=identification.get('artist_name'),
image_url=identification.get('image_url'),
error_message='Could not match tracks to album tracklist')
self._stats['failed'] += 1
continue
confidence = match_result['confidence']
status = 'matched'
# Check if individual track matches are strong even if overall confidence
# is low (e.g. only 2 of 18 album tracks present → low coverage kills
# overall score, but the 2 tracks match perfectly and should still import)
high_conf_matches = [m for m in match_result.get('matches', []) if m['confidence'] >= 0.8]
has_strong_individual_matches = len(high_conf_matches) > 0
if (confidence >= threshold or has_strong_individual_matches) and auto_process:
# Phase 5: Auto-process — insert an in-progress row
# so the UI sees the import the moment it starts,
# then update it with the final status when done.
effective_conf = max(confidence, min(m['confidence'] for m in high_conf_matches) if high_conf_matches else 0)
logger.info(f"[Auto-Import] Processing {candidate.name}"
f"overall: {confidence:.0%}, {len(high_conf_matches)} strong matches, "
f"{match_result.get('matched_count', 0)}/{match_result.get('total_tracks', '?')} tracks")
in_progress_row_id = self._record_in_progress(
candidate, identification, match_result,
)
self._current_status = 'processing'
success = self._process_matches(candidate, identification, match_result)
status = 'completed' if success else 'failed'
confidence = max(confidence, effective_conf)
if success:
self._stats['auto_processed'] += 1
else:
self._stats['failed'] += 1
# Reset live progress state regardless of outcome
self._current_track_index = 0
self._current_track_total = 0
self._current_track_name = ''
self._current_status = 'scanning' if not self.should_stop else 'idle'
# Update the in-progress row in place — UI shows the
# final result without a separate insert race.
self._finalize_result(in_progress_row_id, status, confidence)
elif confidence >= 0.7:
status = 'pending_review'
self._stats['pending_review'] += 1
logger.info(f"[Auto-Import] Medium confidence ({confidence:.0%}) — pending review: {candidate.name}")
self._record_result(candidate, status, confidence,
album_id=identification.get('album_id'),
album_name=identification.get('album_name'),
artist_name=identification.get('artist_name'),
image_url=identification.get('image_url'),
identification_method=identification.get('method'),
match_data=match_result)
else: else:
status = 'needs_identification' self._bump_stat('failed')
self._stats['failed'] += 1
logger.info(f"[Auto-Import] Low confidence ({confidence:.0%}) — needs manual ID: {candidate.name}")
self._record_result(candidate, status, confidence,
album_id=identification.get('album_id'),
album_name=identification.get('album_name'),
artist_name=identification.get('artist_name'),
image_url=identification.get('image_url'),
identification_method=identification.get('method'),
match_data=match_result)
except Exception as e: # Update the in-progress row in place — UI shows the
logger.error(f"[Auto-Import] Error processing {candidate.name}: {e}") # final result without a separate insert race.
self._record_result(candidate, 'failed', 0.0, error_message=str(e)) self._finalize_result(in_progress_row_id, status, confidence)
self._stats['failed'] += 1 elif confidence >= 0.7:
finally: status = 'pending_review'
self._processing_hashes.discard(candidate.folder_hash) self._bump_stat('pending_review')
# Defensive: if the inner code path didn't reset live logger.info(f"[Auto-Import] Medium confidence ({confidence:.0%}) — pending review: {candidate.name}")
# progress (early raise, etc.), clear it so the UI self._record_result(candidate, status, confidence,
# doesn't show stale "processing track 3/14" forever. album_id=identification.get('album_id'),
self._current_track_index = 0 album_name=identification.get('album_name'),
self._current_track_total = 0 artist_name=identification.get('artist_name'),
self._current_track_name = '' image_url=identification.get('image_url'),
identification_method=identification.get('method'),
match_data=match_result)
else:
status = 'needs_identification'
self._bump_stat('failed')
logger.info(f"[Auto-Import] Low confidence ({confidence:.0%}) — needs manual ID: {candidate.name}")
self._record_result(candidate, status, confidence,
album_id=identification.get('album_id'),
album_name=identification.get('album_name'),
artist_name=identification.get('artist_name'),
image_url=identification.get('image_url'),
identification_method=identification.get('method'),
match_data=match_result)
# Rate limit between folders except Exception as e:
if self._interruptible_sleep(2): logger.error(f"[Auto-Import] Error processing {candidate.name}: {e}")
break self._record_result(candidate, 'failed', 0.0, error_message=str(e))
self._bump_stat('failed')
finally:
with self._submitted_lock:
self._submitted_hashes.discard(candidate.folder_hash)
# Per-candidate UI state goes away with the candidate.
# No stale "processing track 3/14" because the entry is
# gone — the UI's polling read returns an empty array.
self._unregister_active(candidate.folder_hash)
# ── Scanning ── # ── Scanning ──
@ -1233,9 +1494,14 @@ class AutoImportWorker:
processed = 0 processed = 0
errors = [] errors = []
all_matches = list(match_result.get('matches', [])) all_matches = list(match_result.get('matches', []))
# Ensure an active-import entry exists for this candidate.
# Callers from `_process_one_candidate` already registered, but
# tests invoke `_process_matches` directly without going
# through the pool — the auto-register makes both paths safe.
self._register_active(candidate, status='processing')
# Surface track total for the UI's live-progress widget. Matches # Surface track total for the UI's live-progress widget. Matches
# the loop denominator so users see "3/14" while it's working. # the loop denominator so users see "3/14" while it's working.
self._current_track_total = len(all_matches) self._update_active(candidate.folder_hash, track_total=len(all_matches))
for index, match in enumerate(all_matches, start=1): for index, match in enumerate(all_matches, start=1):
track = match['track'] track = match['track']
@ -1249,8 +1515,11 @@ class AutoImportWorker:
# Update live progress BEFORE the per-track work so the UI # Update live progress BEFORE the per-track work so the UI
# sees the right "now processing track N: <name>" the # sees the right "now processing track N: <name>" the
# moment polling fires (every 5s). # moment polling fires (every 5s).
self._current_track_index = index self._update_active(
self._current_track_name = track_name candidate.folder_hash,
track_index=index,
track_name=track_name,
)
if not os.path.exists(file_path): if not os.path.exists(file_path):
errors.append(f"File not found: {os.path.basename(file_path)}") errors.append(f"File not found: {os.path.basename(file_path)}")

View file

@ -0,0 +1,511 @@
"""Pin the bounded-executor + scan-lock concurrency model in
``AutoImportWorker``.
Pre-refactor (before 2026-05-09): manual "Scan Now" clicks spawned a
fresh `threading.Thread(target=_scan_cycle)` per click on top of the
worker's existing 60-second timer-driven scan. Emergent parallelism
with no upper bound, no shared queue, no graceful shutdown. Different
scan cycles raced on `_processing_paths` / `_folder_snapshots` state.
Post-refactor:
- ONE scan at a time (`_scan_lock` non-blocking acquire duplicate
triggers no-op).
- Per-candidate processing runs on a `ThreadPoolExecutor` (default 3
workers, configurable via `auto_import.max_workers`).
- Both timer + manual triggers share `trigger_scan()` so they go
through the same lock + executor.
These tests pin the CONCURRENCY CONTRACT, not the per-candidate
processing logic (which is covered separately by
``test_auto_import_live_progress.py`` etc.).
"""
from __future__ import annotations
import threading
import time
from unittest.mock import MagicMock, patch
import pytest
from core.auto_import_worker import AutoImportWorker, FolderCandidate
def _make_worker(max_workers: int = 3) -> AutoImportWorker:
"""Bare worker — for the executor/lock tests we don't need full
db / config / process_callback dependencies."""
return AutoImportWorker(
database=MagicMock(),
process_callback=MagicMock(),
max_workers=max_workers,
)
def _make_candidate(folder_hash: str = 'h1', name: str = 'TestAlbum') -> FolderCandidate:
return FolderCandidate(
path=f'/staging/{name}',
name=name,
audio_files=[f'/staging/{name}/01.flac'],
folder_hash=folder_hash,
)
# ---------------------------------------------------------------------------
# Pool configuration
# ---------------------------------------------------------------------------
def test_default_max_workers_is_three():
"""Match the existing pool patterns in this codebase
(missing_download_executor, sync_executor, import_singles_executor
all default to 3)."""
w = _make_worker()
assert w._max_workers == 3
def test_max_workers_configurable_via_constructor():
w = _make_worker(max_workers=5)
assert w._max_workers == 5
def test_max_workers_floors_at_one():
"""0 or negative pool size would deadlock anything submitted —
floor at 1 so a misconfigured value still works."""
w = _make_worker(max_workers=0)
assert w._max_workers == 1
def test_max_workers_pulled_from_config_when_provided():
config = MagicMock()
config.get = MagicMock(side_effect=lambda key, default: 7 if key == 'auto_import.max_workers' else default)
w = AutoImportWorker(
database=MagicMock(),
process_callback=MagicMock(),
config_manager=config,
max_workers=3, # constructor default — overridden by config
)
assert w._max_workers == 7
# ---------------------------------------------------------------------------
# Scan lock — duplicate triggers no-op
# ---------------------------------------------------------------------------
def test_concurrent_triggers_only_one_scan_runs(monkeypatch):
"""Pre-refactor regression case: hitting "Scan Now" 5× in quick
succession used to spawn 5 parallel scan cycles. Post-refactor:
only one runs, the rest no-op via the non-blocking lock."""
w = _make_worker()
scan_count = 0
scan_started = threading.Event()
scan_can_finish = threading.Event()
def fake_scan_and_submit():
nonlocal scan_count
scan_count += 1
scan_started.set()
scan_can_finish.wait(timeout=5)
monkeypatch.setattr(w, '_scan_and_submit', fake_scan_and_submit)
# Fire 5 trigger_scan calls in parallel
threads = [threading.Thread(target=w.trigger_scan) for _ in range(5)]
for t in threads:
t.start()
# Wait for the first scan to start
assert scan_started.wait(timeout=5)
# The other 4 should have already returned (lock was held)
time.sleep(0.1)
assert scan_count == 1, (
f"Expected exactly 1 scan to run while the lock was held, got "
f"{scan_count}. The non-blocking scan lock isn't gating "
f"duplicate triggers."
)
# Release the held scan
scan_can_finish.set()
for t in threads:
t.join(timeout=5)
# No additional scans started after release (the 4 losers gave up,
# didn't queue)
assert scan_count == 1
def test_scan_after_previous_finishes_runs_normally(monkeypatch):
"""Lock releases when scan finishes — next trigger should acquire
+ run normally, not be permanently blocked."""
w = _make_worker()
scan_count = 0
def fake_scan_and_submit():
nonlocal scan_count
scan_count += 1
monkeypatch.setattr(w, '_scan_and_submit', fake_scan_and_submit)
w.trigger_scan()
w.trigger_scan()
w.trigger_scan()
assert scan_count == 3
# ---------------------------------------------------------------------------
# Executor — per-candidate parallelism
# ---------------------------------------------------------------------------
def test_candidates_dispatched_to_executor(monkeypatch):
"""Scan finds N candidates → submits N tasks to the executor pool.
Pool runs them in parallel (up to max_workers). Each task ends up
calling `_process_one_candidate`."""
w = _make_worker(max_workers=3)
w.start() # initialises the executor
try:
candidates = [
_make_candidate(folder_hash=f'h{i}', name=f'Album{i}')
for i in range(5)
]
monkeypatch.setattr(w, '_enumerate_folders', lambda staging: candidates)
monkeypatch.setattr(w, '_resolve_staging_path', lambda: '/staging')
monkeypatch.setattr('core.auto_import_worker.os.path.isdir', lambda p: True)
monkeypatch.setattr(w, '_is_already_processed', lambda h: False)
monkeypatch.setattr(w, '_is_folder_stable', lambda c: True)
processed = []
processed_lock = threading.Lock()
def fake_process(candidate):
with processed_lock:
processed.append(candidate.folder_hash)
monkeypatch.setattr(w, '_process_one_candidate', fake_process)
w.trigger_scan()
# Wait for all 5 to finish (executor runs async)
deadline = time.time() + 5
while len(processed) < 5 and time.time() < deadline:
time.sleep(0.05)
assert sorted(processed) == [f'h{i}' for i in range(5)]
finally:
w.stop()
def test_pool_runs_candidates_in_parallel():
"""With max_workers=3, the pool should run up to 3 candidates
concurrently proves the bounded parallelism the user asked for."""
w = _make_worker(max_workers=3)
w.start()
try:
# Submit 3 long-running tasks directly to the executor and
# confirm they run concurrently.
in_flight = [0]
peak_in_flight = [0]
lock = threading.Lock()
proceed = threading.Event()
def slow_task():
with lock:
in_flight[0] += 1
if in_flight[0] > peak_in_flight[0]:
peak_in_flight[0] = in_flight[0]
proceed.wait(timeout=2)
with lock:
in_flight[0] -= 1
futures = [w._executor.submit(slow_task) for _ in range(3)]
# Give them a beat to start
time.sleep(0.2)
assert peak_in_flight[0] == 3, (
f"Expected 3 concurrent tasks, peaked at {peak_in_flight[0]}"
)
proceed.set()
for f in futures:
f.result(timeout=2)
finally:
w.stop()
def test_executor_max_workers_caps_concurrency():
"""max_workers=2 must NOT allow 3 concurrent tasks. Bounded
parallelism predictable system load."""
w = _make_worker(max_workers=2)
w.start()
try:
in_flight = [0]
peak = [0]
lock = threading.Lock()
proceed = threading.Event()
def slow_task():
with lock:
in_flight[0] += 1
if in_flight[0] > peak[0]:
peak[0] = in_flight[0]
proceed.wait(timeout=2)
with lock:
in_flight[0] -= 1
futures = [w._executor.submit(slow_task) for _ in range(5)]
time.sleep(0.3)
assert peak[0] == 2, (
f"max_workers=2 should cap concurrency at 2, peaked at {peak[0]}"
)
proceed.set()
for f in futures:
f.result(timeout=2)
finally:
w.stop()
# ---------------------------------------------------------------------------
# Submitted-hashes dedup across triggers
# ---------------------------------------------------------------------------
def test_candidate_only_submitted_once_across_concurrent_scans(monkeypatch):
"""Scenario: scan A submits candidate X to the pool; pool worker
is mid-processing. Scan B (manual trigger) enumerates again and
sees X must NOT re-submit. `_submitted_hashes` set + lock
prevents double-submission."""
w = _make_worker()
w.start()
try:
cand = _make_candidate(folder_hash='shared-hash')
monkeypatch.setattr(w, '_enumerate_folders', lambda staging: [cand])
monkeypatch.setattr(w, '_resolve_staging_path', lambda: '/staging')
monkeypatch.setattr('core.auto_import_worker.os.path.isdir', lambda p: True)
monkeypatch.setattr(w, '_is_already_processed', lambda h: False)
monkeypatch.setattr(w, '_is_folder_stable', lambda c: True)
process_count = 0
process_lock = threading.Lock()
process_can_finish = threading.Event()
def slow_process(candidate):
nonlocal process_count
with process_lock:
process_count += 1
process_can_finish.wait(timeout=5)
monkeypatch.setattr(w, '_process_one_candidate', slow_process)
# First scan submits the candidate
w.trigger_scan()
# Wait for processing to start
time.sleep(0.1)
# Second scan WHILE first is processing — must not re-submit
w.trigger_scan()
time.sleep(0.1)
assert process_count == 1, (
f"Expected only 1 process call (dedup active), got {process_count}"
)
process_can_finish.set()
time.sleep(0.2)
# After the first finishes, the candidate still has the same
# hash + would be `_is_already_processed`, but our mock returns
# False — even so, the post-finally `discard` should let a
# third trigger re-pick if needed. Here we just verify dedup
# held while in flight.
finally:
process_can_finish.set()
w.stop()
# ---------------------------------------------------------------------------
# Graceful shutdown
# ---------------------------------------------------------------------------
def test_stop_waits_for_inflight_pool_work():
"""`stop()` must call `executor.shutdown(wait=True)` so in-flight
file moves / tag writes / DB inserts complete before shutdown
reports done. Otherwise interrupted writes corrupt state."""
w = _make_worker()
w.start()
finished = threading.Event()
def slow_task():
time.sleep(0.3)
finished.set()
w._executor.submit(slow_task)
# Stop immediately — should block until slow_task completes
w.stop()
assert finished.is_set(), (
"stop() returned before in-flight pool work finished — "
"executor shutdown(wait=True) is missing or broken"
)
# ---------------------------------------------------------------------------
# Per-candidate state isolation under parallel pool workers
# ---------------------------------------------------------------------------
#
# Pre-refactor `_current_folder` / `_current_track_*` / `_current_status` were
# scalar fields on the worker. Three pool workers running in parallel would
# stomp each other's values — UI showed "Processing AlbumA, track 7/14:
# SongFromAlbumB" interleaved garbage. These tests pin the per-candidate
# isolation introduced by the `_active_imports` dict + `_active_lock`.
def test_concurrent_candidates_dont_stomp_each_other():
"""Two pool workers updating their own candidate state must not
interfere each candidate's track_index / track_name / folder_name
is read back exactly as written for that hash."""
w = _make_worker(max_workers=2)
w.start()
try:
cand_a = _make_candidate(folder_hash='hA', name='AlbumA')
cand_b = _make_candidate(folder_hash='hB', name='AlbumB')
# Register both
w._register_active(cand_a, status='processing')
w._register_active(cand_b, status='processing')
ready = threading.Barrier(2)
done = threading.Event()
def worker_for(cand, name_prefix, total):
ready.wait(timeout=2)
for i in range(1, total + 1):
w._update_active(
cand.folder_hash,
track_index=i,
track_total=total,
track_name=f'{name_prefix}-track-{i}',
)
# Tight loop so the two threads interleave aggressively
time.sleep(0.001)
ta = threading.Thread(target=worker_for, args=(cand_a, 'A', 50))
tb = threading.Thread(target=worker_for, args=(cand_b, 'B', 50))
ta.start(); tb.start()
ta.join(timeout=5); tb.join(timeout=5)
done.set()
snap = w._snapshot_active()
by_hash = {a['folder_hash']: a for a in snap}
assert by_hash['hA']['folder_name'] == 'AlbumA', (
"Candidate A's folder_name was overwritten by a parallel candidate — "
f"got {by_hash['hA']['folder_name']!r}"
)
assert by_hash['hB']['folder_name'] == 'AlbumB', (
"Candidate B's folder_name was overwritten — "
f"got {by_hash['hB']['folder_name']!r}"
)
assert by_hash['hA']['track_index'] == 50
assert by_hash['hB']['track_index'] == 50
assert by_hash['hA']['track_name'].startswith('A-')
assert by_hash['hB']['track_name'].startswith('B-')
finally:
w.stop()
def test_get_status_returns_coherent_active_imports_array():
"""`get_status()` must return one entry per in-flight candidate
with the right per-candidate fields the polling UI reads this
array to render multiple in-flight imports simultaneously."""
w = _make_worker(max_workers=3)
w.start()
try:
for i, name in enumerate(['One', 'Two', 'Three']):
cand = _make_candidate(folder_hash=f'h{i}', name=name)
w._register_active(cand, status='processing')
w._update_active(cand.folder_hash, track_index=i + 1, track_total=10)
status = w.get_status()
active = status.get('active_imports') or []
assert len(active) == 3
names = {a['folder_name'] for a in active}
assert names == {'One', 'Two', 'Three'}
# Aggregate top-level should be 'processing' (any active is
# processing → processing wins)
assert status['current_status'] == 'processing'
# Legacy single-import scalars: populated from the FIRST
# active entry (insertion order) so the existing UI keeps
# working when only one candidate is in flight.
assert status['current_folder'] == 'One'
assert status['current_track_index'] == 1
assert status['current_track_total'] == 10
finally:
w.stop()
def test_unregister_removes_only_that_candidate():
"""`_unregister_active(hash)` removes one entry; others stay
visible. Pool workers finishing in any order must not affect
other in-flight candidates' UI state."""
w = _make_worker()
w.start()
try:
for i, name in enumerate(['X', 'Y', 'Z']):
w._register_active(_make_candidate(folder_hash=f'k{i}', name=name))
w._unregister_active('k1')
snap = w._snapshot_active()
names = {a['folder_name'] for a in snap}
assert names == {'X', 'Z'}, f"Unexpected snapshot after unregister: {snap}"
finally:
w.stop()
# ---------------------------------------------------------------------------
# Stats counter integrity under parallel bumps
# ---------------------------------------------------------------------------
def test_stats_increments_are_thread_safe():
"""`self._stats[k] += 1` from multiple threads is read-modify-
write under load the counters drift. `_bump_stat` wraps every
mutation in `_stats_lock` so 1000 parallel bumps land at 1000."""
w = _make_worker()
iterations = 200
threads_count = 5
expected = iterations * threads_count
def hammer():
for _ in range(iterations):
w._bump_stat('scanned')
threads = [threading.Thread(target=hammer) for _ in range(threads_count)]
for t in threads:
t.start()
for t in threads:
t.join(timeout=5)
assert w._stats['scanned'] == expected, (
f"Lost increments: expected {expected}, got {w._stats['scanned']}. "
f"Stats counter is not thread-safe."
)
def test_get_status_stats_snapshot_is_consistent():
"""`get_status()` reads stats under the same lock that mutations
use, so the returned snapshot can't show a partial mid-update
state. Verify the snapshot is a copy (not a live reference)."""
w = _make_worker()
w._bump_stat('scanned')
snap = w.get_status()['stats']
snap['scanned'] = 9999
# Mutating the snapshot must not affect the worker's internal stats
assert w._stats['scanned'] == 1, (
"get_status() returned a live reference to _stats — "
"callers can corrupt internal state."
)

View file

@ -34349,14 +34349,38 @@ def auto_import_reject(item_id):
@app.route('/api/auto-import/scan-now', methods=['POST']) @app.route('/api/auto-import/scan-now', methods=['POST'])
def auto_import_scan_now(): def auto_import_scan_now():
"""Trigger an immediate scan cycle.""" """Trigger an immediate scan cycle.
Routes through `trigger_scan()`, the canonical entry point shared
with the worker's timer loop. Pre-refactor this endpoint spawned
a fresh `_scan_cycle` thread per click emergent parallelism
that grew unbounded with each click and produced racy access to
candidate-tracking state. Post-refactor:
- Manual triggers + the timer loop share one scan-lock, so only
one scan runs at a time
- Per-candidate processing happens on the worker's bounded
`ThreadPoolExecutor` (default 3 workers predictable
concurrency, configurable via `auto_import.max_workers`)
- Multiple "Scan Now" clicks while a scan is in flight no-op
instead of stacking up parallel scanners
Runs the scan in a background thread so the HTTP response returns
immediately `trigger_scan()` itself is fast (just enumeration +
submit), but a slow filesystem walk on a large staging dir could
still hold the request thread for seconds. Detached thread is
safe: scan-lock prevents duplicate work, executor handles
per-candidate processing.
"""
if not auto_import_worker: if not auto_import_worker:
return jsonify({"success": False, "error": "Auto-import not available"}), 500 return jsonify({"success": False, "error": "Auto-import not available"}), 500
if not auto_import_worker.running: if not auto_import_worker.running:
return jsonify({"success": False, "error": "Auto-import is not running"}), 400 return jsonify({"success": False, "error": "Auto-import is not running"}), 400
# Run scan in background thread threading.Thread(
import threading target=auto_import_worker.trigger_scan,
threading.Thread(target=auto_import_worker._scan_cycle, daemon=True).start() daemon=True,
name='AutoImportScanNow',
).start()
return jsonify({"success": True}) return jsonify({"success": True})

View file

@ -3416,6 +3416,7 @@ const WHATS_NEW = {
'2.4.4': [ '2.4.4': [
// --- post-2.4.3 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- // --- post-2.4.3 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.4 dev cycle' }, { date: 'Unreleased — 2.4.4 dev cycle' },
{ title: 'Auto-Import: Process Multiple Albums At Once', desc: 'auto-import used to process one album at a time. drop 5 albums into staging → wait for the first to fully finish (identify + match + every track post-processed) before the second one even starts. on a slow network or with a big batch this means 30+ minutes of staring at "Processing AlbumOne" while the others sit untouched. now there\'s a small bounded thread pool (3 workers by default, configurable) — up to 3 albums process in parallel, the queue moves through the rest as workers free up. clicking "Scan Now" multiple times no longer spawns extra unbounded scan threads — every trigger (timer + manual button) routes through one shared scan lock so duplicate triggers no-op instead of stacking up. live progress widget on the auto-import card now lists EACH in-flight album with its own track index/total/name instead of one shared scalar that the parallel workers used to stomp on each other. graceful shutdown: stopping the worker waits for in-flight pool work to finish before reporting stopped — no half-moved files or partial DB writes mid-album. stats counters (`scanned` / `auto_processed` / `pending_review` / `failed`) now use a lock so parallel workers don\'t lose increments under load. 17 new tests pin: pool size config, scan lock dedup, executor dispatch + bounded parallelism, cross-trigger candidate dedup, graceful shutdown, per-candidate UI state isolation across parallel workers, stats counter thread-safety, and snapshot consistency.', page: 'import' },
{ title: 'Manual Search In The Failed-Track Candidates Modal', desc: 'when a download fails or returns "not found" the user can already click the status cell to open a modal showing whatever search candidates the auto-search left over and pick a different one. that modal now ALSO has a manual search bar. type any query, hit search, get a fresh round of results from the download sources without having to start the whole download flow over from the search page. solves the case where the auto-query was bad (featured artist not in title, parentheticals like "(remastered 2019)" tripping the matcher, slight artist-name variants) but the file genuinely exists on the source. source picker is smart per download mode: single-source mode (soulseek-only / youtube-only / etc) shows a "searching X" label, no dropdown; hybrid mode shows a dropdown with "all sources" default plus every configured source — picking "all" runs parallel searches across all of them and tags each result row with its source badge. only configured sources show up; unconfigured ones are hidden. results stream in as each source completes via NDJSON instead of blocking on the slowest source — the table starts populating the moment the first source returns. clicking a result reuses the existing retry-download flow → same path, same acoustid verification on the file when it lands, no shortcut around the safety net. additive in the truest sense: the existing modal layout / candidates table / download buttons are byte-identical when the user doesn\'t use manual search. backend extends the candidates endpoint with `download_mode` + `available_sources` + a `source` field per candidate (purely additive — old fields untouched), and adds a new `POST /api/downloads/task/<id>/manual-search` that streams NDJSON (one header line, one source_results line per source as completed, one done terminator) so the frontend renderer can append rows incrementally. 11 tests pin the streaming contract: query length / source whitelist / task 404 validation, single-source dispatch, parallel "all" dispatch, one-event-per-source streaming shape, unconfigured-source skip + reject, header metadata, and per-source exception isolation (one source raising emits a `source_error` event but doesn\'t fail the stream).', page: 'downloads' }, { title: 'Manual Search In The Failed-Track Candidates Modal', desc: 'when a download fails or returns "not found" the user can already click the status cell to open a modal showing whatever search candidates the auto-search left over and pick a different one. that modal now ALSO has a manual search bar. type any query, hit search, get a fresh round of results from the download sources without having to start the whole download flow over from the search page. solves the case where the auto-query was bad (featured artist not in title, parentheticals like "(remastered 2019)" tripping the matcher, slight artist-name variants) but the file genuinely exists on the source. source picker is smart per download mode: single-source mode (soulseek-only / youtube-only / etc) shows a "searching X" label, no dropdown; hybrid mode shows a dropdown with "all sources" default plus every configured source — picking "all" runs parallel searches across all of them and tags each result row with its source badge. only configured sources show up; unconfigured ones are hidden. results stream in as each source completes via NDJSON instead of blocking on the slowest source — the table starts populating the moment the first source returns. clicking a result reuses the existing retry-download flow → same path, same acoustid verification on the file when it lands, no shortcut around the safety net. additive in the truest sense: the existing modal layout / candidates table / download buttons are byte-identical when the user doesn\'t use manual search. backend extends the candidates endpoint with `download_mode` + `available_sources` + a `source` field per candidate (purely additive — old fields untouched), and adds a new `POST /api/downloads/task/<id>/manual-search` that streams NDJSON (one header line, one source_results line per source as completed, one done terminator) so the frontend renderer can append rows incrementally. 11 tests pin the streaming contract: query length / source whitelist / task 404 validation, single-source dispatch, parallel "all" dispatch, one-event-per-source streaming shape, unconfigured-source skip + reject, header metadata, and per-source exception isolation (one source raising emits a `source_error` event but doesn\'t fail the stream).', page: 'downloads' },
{ title: 'Manual Picks Don\'t Auto-Retry Anymore (And The Modal Always Opens)', desc: 'three follow-on fixes to the manual-search feature once people started actually using it. (1) when the user picked a candidate and that download failed (e.g. soundcloud 404 on a stale track url), the auto-retry monitor would treat it like any other failed auto-attempt — yank the task back to "searching" and pick a different candidate. felt completely wrong from the user\'s perspective: "i picked THIS one, why is it searching for something else?" now manual picks are tagged with a `_user_manual_pick` flag and the auto-retry path bails on it. failure surfaces to the user instead of getting silently fallen-back. (2) non-soulseek manual picks (youtube / tidal / qobuz / hifi / deezer / soundcloud / lidarr) were getting stuck at "downloading 0%" forever even after their engine reported terminal failure. cause: status polling went into a "let monitor handle retry" branch that never fired because manual picks bail on retry — task was orphaned in downloading state. fix: when the engine reports Errored on a manual pick, mark the task failed directly, don\'t defer to the monitor. plus an engine-state fallback path covers the rare race where the orchestrator\'s pre-populated transfer lookup is missing the entry. (3) failed / not_found rows were only clickable when the auto-search had cached candidates — but the whole point of opening the modal now is to RUN a manual search, which doesn\'t need pre-existing candidates. now every failed / not_found / cancelled row opens the modal regardless. (4) one nasty deadlock fix in the process: the new "mark failed" path was synchronously calling `on_download_completed` while holding `tasks_lock`, which itself re-acquires the same lock — `threading.Lock` is non-reentrant so the polling thread wedged forever. while wedged the lock was held → every other endpoint that needed it (including /candidates → can\'t open OTHER modals) hung waiting. moved completion callbacks onto a daemon thread so the lock releases first. (5) manual download worker now runs on its own dedicated thread instead of competing with the batch\'s 3-worker `missing_download_executor` pool — saturated batches no longer queue manual picks indefinitely. all changes are scoped to manual picks only via the `_user_manual_pick` flag — auto-attempt flow is byte-identical to before. 17 unit tests pin the gate behavior (status engine fallback / monitor retry skip / IF-branch failure transition / auto-attempt skip).', page: 'downloads' }, { title: 'Manual Picks Don\'t Auto-Retry Anymore (And The Modal Always Opens)', desc: 'three follow-on fixes to the manual-search feature once people started actually using it. (1) when the user picked a candidate and that download failed (e.g. soundcloud 404 on a stale track url), the auto-retry monitor would treat it like any other failed auto-attempt — yank the task back to "searching" and pick a different candidate. felt completely wrong from the user\'s perspective: "i picked THIS one, why is it searching for something else?" now manual picks are tagged with a `_user_manual_pick` flag and the auto-retry path bails on it. failure surfaces to the user instead of getting silently fallen-back. (2) non-soulseek manual picks (youtube / tidal / qobuz / hifi / deezer / soundcloud / lidarr) were getting stuck at "downloading 0%" forever even after their engine reported terminal failure. cause: status polling went into a "let monitor handle retry" branch that never fired because manual picks bail on retry — task was orphaned in downloading state. fix: when the engine reports Errored on a manual pick, mark the task failed directly, don\'t defer to the monitor. plus an engine-state fallback path covers the rare race where the orchestrator\'s pre-populated transfer lookup is missing the entry. (3) failed / not_found rows were only clickable when the auto-search had cached candidates — but the whole point of opening the modal now is to RUN a manual search, which doesn\'t need pre-existing candidates. now every failed / not_found / cancelled row opens the modal regardless. (4) one nasty deadlock fix in the process: the new "mark failed" path was synchronously calling `on_download_completed` while holding `tasks_lock`, which itself re-acquires the same lock — `threading.Lock` is non-reentrant so the polling thread wedged forever. while wedged the lock was held → every other endpoint that needed it (including /candidates → can\'t open OTHER modals) hung waiting. moved completion callbacks onto a daemon thread so the lock releases first. (5) manual download worker now runs on its own dedicated thread instead of competing with the batch\'s 3-worker `missing_download_executor` pool — saturated batches no longer queue manual picks indefinitely. all changes are scoped to manual picks only via the `_user_manual_pick` flag — auto-attempt flow is byte-identical to before. 17 unit tests pin the gate behavior (status engine fallback / monitor retry skip / IF-branch failure transition / auto-attempt skip).', page: 'downloads' },
{ title: 'Manual Import: Stop Writing "Unknown Artist / album_id / 0 tracks" Garbage', desc: 'github issue #524 (radoslav-orlov): clicking an album in the import page → all imported albums landed in the library as "Unknown Artist" with the raw 10-digit album id as the title and 0 tracks. cause: the click handler `importPageSelectAlbum(albumId)` was passing only the id to the `/api/import/album/match` POST. the search response carried `source` (which metadata source the album_id came from) + `album_name` + `album_artist`, but the click discarded everything except the id. backend `get_artist_album_tracks` then guessed the source via the configured primary-source priority chain — for a non-deezer-primary user clicking a deezer search result, the chain tries spotify/itunes/discogs first against a deezer numeric id, all return None, and the lookup falls through to the failure-fallback dict (`name = album_id`, no artist field, `total_tracks = 0`). that broken metadata then flowed through the import pipeline → soulsync standalone library got the garbage rows. fix: cache album lookup by id when the suggestions / search renderers run, then have `importPageSelectAlbum` pull `source` + `name` + `artist` from the cache and include them in the match POST. backend now also logs a clear warning when source is missing from the match request, so any future caller dropping it shows up in app.log instead of silently corrupting library imports.', page: 'import' }, { title: 'Manual Import: Stop Writing "Unknown Artist / album_id / 0 tracks" Garbage', desc: 'github issue #524 (radoslav-orlov): clicking an album in the import page → all imported albums landed in the library as "Unknown Artist" with the raw 10-digit album id as the title and 0 tracks. cause: the click handler `importPageSelectAlbum(albumId)` was passing only the id to the `/api/import/album/match` POST. the search response carried `source` (which metadata source the album_id came from) + `album_name` + `album_artist`, but the click discarded everything except the id. backend `get_artist_album_tracks` then guessed the source via the configured primary-source priority chain — for a non-deezer-primary user clicking a deezer search result, the chain tries spotify/itunes/discogs first against a deezer numeric id, all return None, and the lookup falls through to the failure-fallback dict (`name = album_id`, no artist field, `total_tracks = 0`). that broken metadata then flowed through the import pipeline → soulsync standalone library got the garbage rows. fix: cache album lookup by id when the suggestions / search renderers run, then have `importPageSelectAlbum` pull `source` + `name` + `artist` from the cache and include them in the match POST. backend now also logs a clear warning when source is missing from the match request, so any future caller dropping it shows up in app.log instead of silently corrupting library imports.', page: 'import' },

View file

@ -759,26 +759,37 @@ async function _autoImportLoadStatus() {
if (settingsRow) settingsRow.style.display = data.running ? '' : 'none'; if (settingsRow) settingsRow.style.display = data.running ? '' : 'none';
if (scanNowBtn) scanNowBtn.style.display = data.running ? '' : 'none'; if (scanNowBtn) scanNowBtn.style.display = data.running ? '' : 'none';
// Live scan + per-track processing progress // Live scan + per-track processing progress.
// `active_imports` (added when the worker switched to a bounded
// executor pool) is the source of truth; multiple albums can be
// in flight at once. Render each one on its own line; fall back
// to the legacy single-line summary for older backend payloads.
if (progressEl) { if (progressEl) {
if (data.current_status === 'processing') { const active = Array.isArray(data.active_imports) ? data.active_imports : [];
if (active.length > 0) {
progressEl.style.display = ''; progressEl.style.display = '';
if (progressText) { if (progressText) {
const idx = data.current_track_index || 0; const lines = active.map(a => {
const total = data.current_track_total || 0; const folder = a.folder_name || '...';
const trackName = data.current_track_name || ''; const idx = a.track_index || 0;
const folder = data.current_folder || '...'; const total = a.track_total || 0;
if (total > 0) { const trackName = a.track_name || '';
progressText.textContent = `Processing ${folder} — track ${idx}/${total}: ${trackName}`; if (a.status === 'processing' && total > 0) {
} else { return `${folder} — track ${idx}/${total}: ${trackName}`;
progressText.textContent = `Processing: ${folder}`; }
} if (a.status === 'matching') return `${folder} — matching tracks…`;
if (a.status === 'identifying') return `${folder} — identifying…`;
return `${folder} — queued`;
});
progressText.textContent = lines.length === 1
? `Processing ${lines[0]}`
: `Processing ${lines.length} imports:\n${lines.join('\n')}`;
} }
} else if (data.current_status === 'scanning') { } else if (data.current_status === 'scanning') {
progressEl.style.display = ''; progressEl.style.display = '';
if (progressText) { if (progressText) {
const stats = data.stats || {}; const stats = data.stats || {};
progressText.textContent = `Scanning: ${data.current_folder || '...'} (${stats.scanned || 0} processed)`; progressText.textContent = `Scanning (${stats.scanned || 0} processed)`;
} }
} else { } else {
progressEl.style.display = 'none'; progressEl.style.display = 'none';
@ -894,14 +905,19 @@ async function _autoImportLoadResults() {
r.status === 'processing' ? 'processing' : 'neutral'; r.status === 'processing' ? 'processing' : 'neutral';
// Live per-track progress for the row currently being processed. // Live per-track progress for the row currently being processed.
// Match by folder_name since the worker only tracks one folder at a time. // Match by folder_hash through the `active_imports` array
// — the worker now runs multiple imports in parallel via a
// bounded executor pool, so `current_folder` alone can't
// identify a row's live state.
const liveStatus = _autoImportLastStatus; const liveStatus = _autoImportLastStatus;
const liveActive = (liveStatus && Array.isArray(liveStatus.active_imports))
? liveStatus.active_imports.find(a => a.folder_hash === r.folder_hash)
: null;
const isLiveProcessing = r.status === 'processing' const isLiveProcessing = r.status === 'processing'
&& liveStatus && liveStatus.current_status === 'processing' && liveActive && liveActive.status === 'processing';
&& liveStatus.current_folder === r.folder_name; const liveTrackIdx = isLiveProcessing ? (liveActive.track_index || 0) : 0;
const liveTrackIdx = isLiveProcessing ? (liveStatus.current_track_index || 0) : 0; const liveTrackTotal = isLiveProcessing ? (liveActive.track_total || 0) : 0;
const liveTrackTotal = isLiveProcessing ? (liveStatus.current_track_total || 0) : 0; const liveTrackName = isLiveProcessing ? (liveActive.track_name || '') : '';
const liveTrackName = isLiveProcessing ? (liveStatus.current_track_name || '') : '';
// Parse match data for track details // Parse match data for track details
let matchCount = 0, totalTracks = 0, trackDetails = []; let matchCount = 0, totalTracks = 0, trackDetails = [];