perf(scan): skip full library re-stat when the tree is unchanged (#979)

* perf(scan): skip full library re-stat when the tree is unchanged

Startup scans globbed the whole DLC tree twice (*.feedpak, *.wem) and
stat()'d every file to detect changes — ~100k filesystem round trips on
a 50k-song library, and painful on a slow NTFS-3G FUSE mount (the "big
drive churns on every launch" report).

Adds/removes/renames of songs all bump the mtime of the containing
directory (verified on the target mount), so after a full pass we persist
{reldir: mtime_ns} for every library dir (scan_dir_signature.json, keyed
by DLC path). The next scan re-stats only those dirs — a handful vs 100k
ops — and skips the entire listing/stat pass when none changed.

Blind spot: a pack rewritten in place under the same name bumps the file
mtime but not its dir's. Rare for a song library, and the manual Refresh
(/api/rescan + /api/rescan/full) now passes force=True to always do the
full pass. force threads through kick_scan -> _scan_runner and coalesces
like the rescan-pending flag.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(scan): track directory-form songs' own dir in the signature

CodeRabbit: _library_dirs recorded only each song's parent. For a
directory-form song (loose-song folder or directory sloppak bundle),
adding/removing/replacing a file INSIDE the folder bumps that folder's
own mtime, not its parent's — so the fast path would skip a rescan it
should run. Record the song's own dir when f.is_dir(). File-form
sloppaks (a single .feedpak zip) aren't dirs, so the flat file library
is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

lib/scan.py

@@ -51,6 +51,97 @@ from scan_worker import _relpath, _scan_one
 
 log = logging.getLogger("feedBack.scan")
 
+import json
+
+
+# ── Directory-signature fast path ─────────────────────────────────────────────
+#
+# A startup scan globs the whole library twice (*.feedpak, *.wem) and stats every
+# file to detect what changed. On a 50k-song library that lives on a slow mount
+# (an NTFS-3G FUSE volume here) it is ~100k filesystem round trips every launch —
+# the "big drive churns on every startup" report.
+#
+# But adds / removes / renames of songs all bump the mtime of the DIRECTORY that
+# holds them (verified on the target NTFS-3G mount), and so does the addition of
+# a subdirectory (a new entry in its parent). So after a scan we record every
+# library directory and its mtime; on the next scan we re-stat ONLY those
+# directories (a handful, vs 100k file ops). If none changed, the file set is
+# unchanged and the whole listing/stat pass is skipped.
+#
+# The one thing this cannot see is a file edited IN PLACE under the same name —
+# that bumps the file's mtime but not its directory's. That is rare for a song
+# library (you add and remove packs, you don't rewrite them under the same name),
+# and the manual Refresh forces a full scan (force=True) for exactly that case.
+def _dir_signature_file() -> Path:
+    return appstate.config_dir / "scan_dir_signature.json"
+
+
+def _load_dir_signature() -> dict | None:
+    try:
+        data = json.loads(_dir_signature_file().read_text(encoding="utf-8"))
+        if isinstance(data, dict) and isinstance(data.get("dirs"), dict):
+            return data
+    except (OSError, ValueError):
+        pass
+    return None
+
+
+def _save_dir_signature(dlc: Path, dirs: dict[str, int]) -> None:
+    # Keyed by the DLC path so switching libraries never matches a stale
+    # signature. Best-effort: a failed write just means the next scan is a full
+    # one, never a wrong one.
+    try:
+        _dir_signature_file().write_text(
+            json.dumps({"dlc": str(dlc), "dirs": dirs}), encoding="utf-8")
+    except OSError as e:
+        log.debug("scan: could not persist dir signature: %s", e)
+
+
+def _library_dirs(all_songs, dlc: Path) -> set[str]:
+    """Every directory whose mtime reflects an add/remove of a library song:
+    each song's containing directory and all of its ancestors up to the DLC
+    root (the root itself always included, as "."). Derived from the already-
+    listed songs — no extra filesystem walk. The builtin carve-outs
+    (tutorials-builtin / minigames-builtin) are absent because the caller
+    already excluded them from `all_songs`, so a minigame writing a drill there
+    never invalidates the fast path.
+
+    Directory-form songs (loose-song folders, directory sloppak bundles) also
+    record their OWN directory: a file added/removed/replaced INSIDE the folder
+    bumps that folder's mtime but not its parent's, so tracking only the parent
+    would miss an in-place change to such a song. File-form sloppaks (a single
+    .feedpak zip) aren't dirs, so they add nothing here — the flat file library
+    stays at a handful of dir stats."""
+    rels = {"."}
+    for f in all_songs:
+        rel = Path(_relpath(f, dlc))
+        if f.is_dir():
+            rels.add(rel.as_posix())
+        parent = rel.parent
+        rels.add(parent.as_posix())
+        for anc in parent.parents:
+            rels.add(anc.as_posix())
+    return rels
+
+
+def _record_dir_signature(all_songs, dlc: Path) -> None:
+    sig = _stat_dirs(dlc, _library_dirs(all_songs, dlc))
+    if sig is not None:   # a dir vanished mid-scan → skip; next scan is full
+        _save_dir_signature(dlc, sig)
+
+
+def _stat_dirs(dlc: Path, rels) -> dict[str, int] | None:
+    """{reldir: mtime_ns} for the given library dirs, or None if any is gone or
+    unreadable — a vanished recorded dir means the tree changed, so fail to a
+    full scan rather than a false match."""
+    out: dict[str, int] = {}
+    for rel in rels:
+        try:
+            out[rel] = (dlc if rel == "." else dlc / rel).stat().st_mtime_ns
+        except OSError:
+            return None
+    return out
+
 
 _SCAN_STATUS_INIT = {"running": False, "stage": "idle", "total": 0, "done": 0, "current": "", "error": None, "is_first_scan": False, "added": 0, "removed": 0}
 
@@ -99,9 +190,12 @@ def _make_scan_executor():
     )
 
 
-def background_scan():
+def background_scan(force: bool = False):
     """Scan the library and cache song metadata on startup. Uses a process pool to bypass the GIL for CPU-bound metadata parsing.
 
+    `force` skips the directory-signature fast path and always does the full
+    listing/stat pass — the manual Refresh sets it (see _dir_signature_file).
+
     Never sets `_scan_status["running"] = False` — ownership of that flag
     lives in `_scan_runner` so a `kick_scan()` racing this function's
     terminal write cannot observe a stale False and start a second runner.
@@ -121,6 +215,22 @@ def background_scan():
     builtin_content.seed_builtin_diagnostic_sloppaks(appstate.server_root, dlc)
     builtin_content.seed_builtin_starter_content(appstate.server_root, dlc)
 
+    # Fast path: if every library directory recorded by the last scan still has
+    # the same mtime, nothing was added, removed, or renamed, so the whole
+    # glob-and-stat pass below can be skipped (see the signature comment above).
+    # `force` (manual Refresh) always does the full pass. Seeding above is
+    # idempotent — it only writes when a builtin is missing — so it does not
+    # perturb the mtimes on a settled library.
+    if not force:
+        stored = _load_dir_signature()
+        if stored is not None and stored.get("dlc") == str(dlc):
+            current = _stat_dirs(dlc, stored["dirs"].keys())
+            if current is not None and current == stored["dirs"]:
+                _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete"}
+                log.info("Scan: library tree unchanged (%d dirs) — skipped the full listing/stat pass",
+                         len(current))
+                return
+
     # Listing can fail on macOS without Full Disk Access, or on Docker if the
     # path isn't shared. Report the failure explicitly rather than silently
     # appearing to scan nothing.
@@ -223,6 +333,9 @@ def background_scan():
             to_scan.append((f, mtime, size, dlc))
 
     if not to_scan:
+        # Full pass completed with the DB already up to date — record the tree
+        # signature so the next startup can take the fast path.
+        _record_dir_signature(all_songs, dlc)
         _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}
         log.info("Scan: nothing new to scan (%d songs, all cached)", len(all_songs))
         return
@@ -247,6 +360,9 @@ def background_scan():
             _scan_status["done"] += 1
             _scan_status["current"] = fname
 
+    # Record the tree signature after a completed full pass so the next startup
+    # can skip it when nothing has changed.
+    _record_dir_signature(all_songs, dlc)
     log.info("Scan complete: %d songs cached", len(to_scan))
     _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}
 
@@ -255,6 +371,9 @@ _scan_kick_lock = threading.Lock()
 
 
 _scan_rescan_pending = False
+# Set by kick_scan(force=True); consumed by _scan_runner for the next pass so a
+# manual Refresh bypasses the directory-signature fast path.
+_scan_force_next = False
 
 
 # Handles to the running scan / enrichment worker threads. Both use the shared
@@ -265,9 +384,15 @@ _scan_rescan_pending = False
 _scan_thread: threading.Thread | None = None
 
 
-def kick_scan() -> bool:
+def kick_scan(force: bool = False) -> bool:
     """Request a library rescan, single-flight + coalescing.
 
+    `force` skips the directory-signature fast path for the resulting pass (the
+    manual Refresh uses it so an in-place same-name edit — the one thing the
+    fast path can't see — is always picked up). A forced request that coalesces
+    onto a running or queued scan keeps the force intent: the pass is forced if
+    ANY pending request asked for it.
+
     Returns True if a new scan thread was started, False if one was already
     running. In the latter case a follow-up pass is queued and runs as soon
     as the current scan finishes so files landing mid-scan (e.g. an upload
@@ -275,8 +400,10 @@ def kick_scan() -> bool:
     until the next periodic pass. Multiple late-arriving requests coalesce
     into a single follow-up.
     """
-    global _scan_rescan_pending, _scan_thread
+    global _scan_rescan_pending, _scan_thread, _scan_force_next
     with _scan_kick_lock:
+        if force:
+            _scan_force_next = True
         if _scan_status["running"]:
             _scan_rescan_pending = True
             return False
@@ -290,10 +417,15 @@ def kick_scan() -> bool:
 
 def _scan_runner():
     """Run _background_scan, then re-run if requests arrived mid-scan."""
-    global _scan_rescan_pending
+    global _scan_rescan_pending, _scan_force_next
     while True:
+        # Consume the force flag for THIS pass; a forced request queued mid-scan
+        # sets it again for the follow-up.
+        with _scan_kick_lock:
+            forced = _scan_force_next
+            _scan_force_next = False
         try:
-            background_scan()
+            background_scan(force=forced)
         except Exception:
             log.exception("background scan failed unexpectedly")
 

server.py

@@ -1115,7 +1115,10 @@ async def startup_status_stream(request: Request):
 @app.post("/api/rescan")
 def trigger_rescan():
     """Manually trigger a library rescan."""
-    if not scan.kick_scan():
+    # force=True: a manual Refresh must skip the directory-signature fast path —
+    # it is the escape hatch for the one change dir mtimes can't see (a pack
+    # rewritten in place under the same name).
+    if not scan.kick_scan(force=True):
         return {"message": "Scan already in progress"}
     return {"message": "Rescan started"}
 
@@ -1133,7 +1136,7 @@ def trigger_full_rescan():
         # delete_missing() prunes anything genuinely gone at the end.
         meta_db.conn.execute("UPDATE songs SET mtime = -1")
         meta_db.conn.commit()
-    if not scan.kick_scan():
+    if not scan.kick_scan(force=True):
         return {"message": "Scan already in progress"}
     return {"message": "Full rescan started"}
 

tests/test_feedpak_extension.py

@@ -137,6 +137,81 @@ def test_background_scan_discovers_both_suffixes(tmp_path, scan_server):
     assert "ignore.zip" not in seen
 
 
+# ── 2b. directory-signature fast path (skip the full re-stat) ────────────────
+
+def test_dir_signature_fast_path_skips_unchanged_tree(tmp_path, scan_server):
+    """After a full scan records the library-dir signature, a second scan with
+    an unchanged tree takes the fast path and does NOT re-glob/extract — but a
+    forced scan (manual Refresh) always does the full pass, and a new song
+    (which bumps the dir mtime) reverts to a full pass on its own."""
+    import unittest.mock as mock
+
+    dlc = tmp_path / "dlc"
+    dlc.mkdir()
+    (dlc / "a.feedpak").write_bytes(b"")
+    (tmp_path / "config.json").write_text('{"dlc_dir": "%s"}' % dlc)
+
+    scan = importlib.import_module("scan")
+    seen: list[str] = []
+
+    def mock_extract(f, dlc_dir):
+        seen.append(f.name)
+        return {"title": f.name, "artist": "", "album": ""}
+
+    with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract):
+        # 1) first pass: full scan, extracts a.feedpak (+ seeded builtins),
+        #    records the signature
+        scan.background_scan()
+        assert "a.feedpak" in seen
+        assert scan._dir_signature_file().exists()
+
+        # 2) unchanged tree: fast path — no glob, no extraction at all
+        seen.clear()
+        scan.background_scan()
+        assert seen == []
+        assert scan.status()["stage"] == "complete"
+
+        # 3) a new song bumps the dlc mtime → signature mismatch → full pass
+        #    picks it up on its own (no manual Refresh needed for adds)
+        (dlc / "b.feedpak").write_bytes(b"")
+        seen.clear()
+        scan.background_scan()
+        assert "b.feedpak" in seen
+
+        # 4) force=True (Refresh) bypasses the fast path even on a settled tree:
+        #    with the signature now current, a plain scan skips, a forced one lists
+        seen.clear()
+        scan.background_scan()          # fast path
+        assert seen == []
+        forced_listed = []
+        real_delete_missing = scan.appstate.meta_db.delete_missing
+        def _spy(files):
+            forced_listed.append(set(files))
+            return real_delete_missing(files)
+        with mock.patch.object(scan.appstate.meta_db, "delete_missing", new=_spy):
+            scan.background_scan(force=True)
+        assert forced_listed, "force=True must run the full listing pass"
+
+
+def test_dir_signature_tracks_directory_form_song_own_dir(tmp_path):
+    """A directory-form song (loose folder / directory bundle) records its OWN
+    directory in the signature, so an in-place file change inside it — which
+    bumps that folder's mtime but not its parent's — invalidates the fast path.
+    A file-form sloppak (a plain .feedpak zip) is not a dir and adds nothing."""
+    scan = importlib.import_module("scan")
+    dlc = tmp_path / "dlc"
+    (dlc / "packs").mkdir(parents=True)
+    loose = dlc / "packs" / "my_loose_song"   # directory-form song
+    loose.mkdir()
+    zipped = dlc / "packs" / "zipped.feedpak"  # file-form song
+    zipped.write_bytes(b"")
+
+    rels = scan._library_dirs([loose, zipped], dlc)
+    assert "packs/my_loose_song" in rels, "directory-form song must track its own dir"
+    assert "packs" in rels and "." in rels
+    assert "packs/zipped.feedpak" not in rels, "a file-form sloppak is not a tracked dir"
+
+
 # ── 3. POST /api/songs/upload gate (endpoint) ────────────────────────────────
 
 @pytest.fixture()