summaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-07 03:13:14 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-07 03:13:14 +0200
commiteeda274d751c537f4ecef3087994a16a9517478f (patch)
tree0435f298010f0f946362f28baebbe88337ca8768 /packages
parente1dbdf0b7bebc27c2da7ae0c7095c5f88d4e1967 (diff)
downloadmeshbay-eeda274d751c537f4ecef3087994a16a9517478f.tar.gz
fix(node): carry enrichment across a rescan instead of re-deriving it
e1dbdf0 made a replugged root re-enrich, which was correct and not enough: the operator still watched their albums vanish. Measured on the reported library with a cold metadata cache, the node broadcast twice — the first delta stripped every album, the second put them back 14 seconds later. Fourteen seconds of "no music found" is the bug, whatever happens after. An entry's id is its content hash, so an entry that comes back under the same id, name and path is the same bytes in the same place and everything enrichment derived from it still holds. `_rescan_root` now carries those fields across the drop-and-rescan that `reconcile` and `plug_root` share. Re-enrichment stays as the fallback for what genuinely changed: a different id is different content, and a different name or path can change the folder and filename fallbacks that artist, album, display_title and track_no rest on, so those entries are still handed to the daemon through `rescanned_ids`. `uploader_id`/`uploader_pk` ride along. They are the same shape of field — set once on an entry, readable from nowhere on disk — and they decide who may delete the file, so losing them to a replug quietly took a right away. Verified on the running node: one broadcast 550ms after the plug, carrying the albums, and no metadata lookups at all. The tests now assert the field on the entry rather than a call to an enricher. Counting calls is what let the previous version of this file pass while the operator still saw an empty tab. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us
Diffstat (limited to 'packages')
-rw-r--r--packages/meshbay-node/src/meshbay_node/indexer/indexer.py66
-rw-r--r--packages/meshbay-node/tests/test_replug_restores_enrichment.py256
2 files changed, 232 insertions, 90 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/indexer/indexer.py b/packages/meshbay-node/src/meshbay_node/indexer/indexer.py
index eea5b4f..33e7210 100644
--- a/packages/meshbay-node/src/meshbay_node/indexer/indexer.py
+++ b/packages/meshbay-node/src/meshbay_node/indexer/indexer.py
@@ -609,8 +609,7 @@ class DirectoryIndexer:
for root, available in changed:
if available:
log.info("Root %r is back — rescanning", root.name)
- self._drop_root_entries(root)
- await self._scan_root(root)
+ await self._rescan_root(root)
touched = True
else:
# Frozen: entries stay, marked unavailable to members through
@@ -711,17 +710,61 @@ class DirectoryIndexer:
return [e for e in self._index.entries
if fold(e.path).split("/", 1)[0] == prefix]
+ # Everything on an IndexEntry that a scan does not produce. `_scan_root`
+ # fills id/name/path/size/type/added_at/hash_version from the file itself;
+ # every field below was derived by one of the enrichment passes and is
+ # nowhere on disk to be read back.
+ _ENRICHED_FIELDS = (
+ "duration", "thumb_hash", "width", "height",
+ "display_title", "season", "episode",
+ "artist", "album", "track_no", "taken_at", "camera",
+ "uploader_id", "uploader_pk",
+ )
+
+ async def _rescan_root(self, root: Root) -> int:
+ """
+ Rebuild one root's entries from disk, keeping what the files still say.
+
+ The two callers — `reconcile` when a root reappears, `plug_root` when
+ the operator plugs one back in — have to re-walk: the drive may have
+ changed while it was away. What they must not do is throw away the
+ enrichment. An entry's id is its content hash, so an entry that comes
+ back under the same id, name and path is the same bytes in the same
+ place, and every field the Videos, Music and Photos passes derived from
+ it still holds. Re-deriving them means minutes of tag reads, ffprobe
+ runs and rate-limited metadata lookups during which the operator's
+ library sits empty — which is exactly what a replug looked like.
+
+ Anything that does *not* match is left bare on purpose: a different id
+ is different content, and a different name or path can change the
+ filename and folder fallbacks that `display_title`, `track_no`,
+ `artist` and `album` fall back to. Those are the entries
+ `daemon._broadcast_index_change` re-enriches, off `rescanned_ids`.
+ """
+ carried = {(e.id, e.name, e.path): e for e in self._entries_under(root)}
+ self._drop_root_entries(root)
+ count = await self._scan_root(root)
+ for entry in self._entries_under(root):
+ old = carried.get((entry.id, entry.name, entry.path))
+ if old is None:
+ continue
+ for field in self._ENRICHED_FIELDS:
+ setattr(entry, field, getattr(old, field))
+ # It came back intact, so it is not one of the entries the daemon
+ # needs to enrich again.
+ self.rescanned_ids.discard(entry.id)
+ return count
+
def _drop_root_entries(self, root: Root) -> None:
"""
- Throw away a root's entries, always in order to rescan it.
+ Throw away a root's entries. Only ever called to rebuild them.
- Both callers — `reconcile` when a root reappears, `plug_root` when the
- operator plugs one back in — rebuild immediately, so nothing outside
- ever observes the gap: no deletion is broadcast, and the entries that
- come back have the same content-hash ids they had before. What they do
- not have is anything enrichment put on them, which is why the ids are
- recorded for `daemon._broadcast_index_change` to re-enrich rather than
- simply forgotten.
+ The ids are recorded because nothing outside can otherwise tell they
+ were rebuilt: no deletion is broadcast (the rescan is immediate) and
+ the entries come back under the same content-hash ids, so a diff
+ against the last broadcast reports neither an addition nor a deletion.
+ `_rescan_root` clears the ones it managed to carry over intact; what is
+ left is genuinely new to the apps and is re-enriched by the daemon.
"""
for entry in self._entries_under(root):
self.rescanned_ids.add(entry.id)
@@ -794,8 +837,7 @@ class DirectoryIndexer:
root.available = root.is_live()
if root.available:
log.info("Root %r plugged — rescanning", root.name)
- self._drop_root_entries(root)
- await self._scan_root(root)
+ await self._rescan_root(root)
self._restart_observer()
self._index.roots = self.roots.describe()
self._index.version = int(time.time())
diff --git a/packages/meshbay-node/tests/test_replug_restores_enrichment.py b/packages/meshbay-node/tests/test_replug_restores_enrichment.py
index 23fc97e..04a06ae 100644
--- a/packages/meshbay-node/tests/test_replug_restores_enrichment.py
+++ b/packages/meshbay-node/tests/test_replug_restores_enrichment.py
@@ -1,36 +1,44 @@
"""
A root that comes back keeps its Videos/Music/Photos metadata.
-Reported live against a real library: a removable root ejected from the
-Files app and plugged back in came back with its files but *without* its
-albums — Music showed "no music found" and stayed that way through a force
-reload, because the loss was on the node, not in the client.
+Reported live: a removable root ejected from the Files app and plugged back
+in returned with its files and without its albums. Music showed "no music
+found", and it did not come back.
-`DirectoryIndexer.plug_root` drops the root's entries and rescans, which is
-right: the drive may have changed while it was away. What it produces is
-bare `IndexEntry` objects — `_hash_or_cached` fills id/name/path/size/type
-and nothing else. Every enrichment field (`artist`, `album`, `track_no`,
-`duration`, `thumb_hash`, `display_title`, `taken_at`, ...) is gone, and
-unlike the video/photo probe results those tag fields are cached nowhere:
-`enrich_audio.py` deliberately re-reads them each time so a rename can
-re-derive the filename fallback. Re-enrichment is the only way back.
+`plug_root` has to re-walk the root — the drive may have changed while it
+was away — and `_scan_root` produces bare entries: `_hash_or_cached` fills
+id/name/path/size/type and nothing else. Every enrichment field went with
+the old object, and the Music tag fields are cached nowhere by design
+(`enrich_audio.py` re-reads them so a rename can re-derive the filename
+fallback).
-Two independent gates then made sure it never ran:
+Two gates then stopped anything from filling them in again:
-* `_broadcast_index_change` schedules enrichment for `delta.additions`.
- Ejecting never broadcasts, so `_last_broadcast_snapshot` still held those
- ids — the re-added entries diffed as *updates*, not additions.
-* `_enrich_new_*_entries` skips anything already in `_enriched_attempted`,
- which is only discarded for `delta.deletions`. Dropping and rescanning
- inside one call means no deletion is ever broadcast, so the mark survived
- a wipe of the very fields it was standing for.
+* enrichment is scheduled for `delta.additions`, and ejecting broadcasts
+ nothing — the last snapshot still held those ids, so the rebuilt entries
+ diffed as *updates*;
+* `_enrich_new_*_entries` skips anything in `_enriched_attempted`, which is
+ only discarded for `delta.deletions` — and dropping and rescanning inside
+ one call broadcasts no deletion either.
-A restart cleared both (an empty snapshot makes every entry an addition),
-which is why this looked like it might fix itself and never did.
+Only a restart cleared both, an empty snapshot making every entry an
+addition. That is why it looked like it might fix itself and never did.
+
+Re-enriching is now the *fallback*, not the fix. An entry's id is its
+content hash, so one that comes back under the same id, name and path is
+the same bytes in the same place and its enrichment still holds:
+`_rescan_root` carries those fields across. Re-deriving them instead meant
+tag reads, ffprobe runs and rate-limited lookups — measured at 14 seconds
+of empty Music tab on a real library with a cold cache, which to the
+operator is indistinguishable from the original bug.
+
+What these assert is therefore the field on the entry, not a call to an
+enricher. Counting calls is what made an earlier version of this file pass
+while the operator still watched their albums vanish.
The same drop-and-rescan runs in `reconcile()` — "Root %r is back" — so a
-USB drive that falls off and returns on its own hits this without anybody
-touching the UI.
+USB drive that falls off and returns on its own hits all of this without
+anybody touching the UI.
"""
import asyncio
@@ -44,6 +52,7 @@ from meshbay_node.config import (Config, GroupConfig, HubConfig, KeystoreConfig,
NodeConfig)
from meshbay_node.daemon import NodeDaemon
from meshbay_node.indexer import DirectoryIndexer
+from meshbay_node.indexer.enrich_audio import AudioEnricher
from meshbay_node.media_cache import MediaCache
from meshbay_node.roots import RootSet
from meshbay_node.roster import Roster
@@ -62,7 +71,7 @@ def _free_port() -> int:
class _CountingEnricher:
- """Stands in for AudioEnricher: records who it was asked to enrich."""
+ """Stands in for an enricher: records who it was asked to enrich."""
def __init__(self):
self.spawned: list[str] = []
@@ -96,94 +105,177 @@ async def _settled(daemon, indexer):
await asyncio.sleep(0.05)
-async def test_a_replugged_root_gets_its_music_metadata_back(tmp_path):
- group_id = "a" * 32
+async def _library(tmp_path, group_id, *, enricher=None):
+ """A one-track library under <root>/<artist>/<album>, enriched once."""
library = tmp_path / "music"
- (library / "an album").mkdir(parents=True)
- (library / "an album" / "track.mp3").write_bytes(_AUDIO_BYTES)
+ (library / "an artist" / "a record").mkdir(parents=True)
+ (library / "an artist" / "a record" / "01 first track.mp3").write_bytes(_AUDIO_BYTES)
daemon = await _daemon(tmp_path, library, group_id)
+ daemon._audio_enricher = enricher or AudioEnricher(daemon._media_cache)
+ await daemon._roster.set_app_directories(
+ group_id, "music", ["music"], set_by="op")
+
+ roots = RootSet.build([{"path": str(library), "removable": True}])
+ indexer = DirectoryIndexer(
+ roots=roots, group_id=group_id,
+ sk_node=Ed25519PrivateKey.generate(), gek=generate_gek())
+ await indexer.initial_scan()
+ await _settled(daemon, indexer)
+ await asyncio.sleep(0.4) # the enricher runs off the broadcast
+ return daemon, indexer, roots, library
+
+
+def _only(indexer):
+ return next(iter(indexer.index.entries))
+
+
+# ── The operator's own eject and plug ────────────────────────────────────────
+
+async def test_the_albums_are_still_there_after_a_replug(tmp_path):
+ """
+ No tags are written: `enrich_audio._artist_album_from_ancestors` derives
+ artist and album from the folder names when a file has none, which is the
+ <library>/<artist>/<album>/<track> layout this was reported against.
+ """
+ group_id = "a" * 32
+ daemon, indexer, _, _ = await _library(tmp_path, group_id)
+ try:
+ before = _only(indexer)
+ assert before.album == "a record" and before.artist == "an artist", (
+ f"the first pass never filled the fields: {before}")
+
+ indexer.eject_root("music")
+ await indexer.plug_root("music")
+ await _settled(daemon, indexer)
+
+ after = _only(indexer)
+ assert after.album == "a record" and after.artist == "an artist", (
+ "the entry came back from the rescan with no album — this is what "
+ "an empty Music tab after a replug looks like on the node")
+ finally:
+ await daemon._media_cache.close()
+ await daemon._roster.close()
+
+
+async def test_the_fields_survive_without_re_deriving_them(tmp_path):
+ """
+ Carried across, not recomputed. Re-deriving is correct and far too slow:
+ on a real library with a cold metadata cache it left Music empty for 14
+ seconds, and an operator who looks in that window sees the bug.
+ """
+ group_id = "b" * 32
enricher = _CountingEnricher()
- daemon._audio_enricher = enricher
+ daemon, indexer, _, _ = await _library(tmp_path, group_id, enricher=enricher)
try:
- await daemon._roster.set_app_directories(
- group_id, "music", ["music/an album"], set_by="op")
+ assert enricher.spawned == ["01 first track.mp3"]
- roots = RootSet.build([{"path": str(library), "removable": True}])
- indexer = DirectoryIndexer(
- roots=roots, group_id=group_id,
- sk_node=Ed25519PrivateKey.generate(), gek=generate_gek())
- await indexer.initial_scan()
+ indexer.eject_root("music")
+ await indexer.plug_root("music")
await _settled(daemon, indexer)
- assert enricher.spawned == ["track.mp3"], "the first pass never ran"
- # The operator ejects the drive from Files, then plugs it back in.
+ assert enricher.spawned == ["01 first track.mp3"], (
+ "an unchanged file was enriched a second time — the whole point "
+ "of the content hash is that it did not need to be")
+ finally:
+ await daemon._media_cache.close()
+ await daemon._roster.close()
+
+
+async def test_who_uploaded_a_file_survives_it_too(tmp_path):
+ """
+ `uploader_id`/`uploader_pk` are the same shape of field — set once, on an
+ entry, readable from nowhere on disk — and they decide who may delete the
+ file. Losing them to a replug quietly takes a right away.
+ """
+ group_id = "c" * 32
+ daemon, indexer, _, _ = await _library(tmp_path, group_id)
+ try:
+ entry = _only(indexer)
+ entry.uploader_id = "alice"
+ entry.uploader_pk = "a-pinned-key"
+
indexer.eject_root("music")
await indexer.plug_root("music")
await _settled(daemon, indexer)
- entry = next(iter(indexer.index.entries))
- assert entry.artist is None and entry.album is None, (
- "the rescan is supposed to produce a bare entry — if this ever "
- "stops being true the rest of this test is measuring nothing")
- assert enricher.spawned == ["track.mp3", "track.mp3"], (
- "a replugged root came back with its files and without its "
- "albums, and nothing was ever going to fill them in again")
+ after = _only(indexer)
+ assert after.uploader_id == "alice" and after.uploader_pk == "a-pinned-key"
finally:
await daemon._media_cache.close()
await daemon._roster.close()
+# ── A drive that leaves and returns on its own ──────────────────────────────
+
async def test_a_root_that_returns_on_its_own_is_treated_the_same(tmp_path):
"""
`reconcile()` rescans a root that reappears without anyone asking — a USB
- drive re-mounting. It goes through the same drop-and-rescan, so it loses
- the same fields, with no click anywhere to blame it on.
+ drive re-mounting. Same drop-and-rescan, so it lost the same fields, with
+ no click anywhere to blame it on.
"""
- group_id = "b" * 32
- library = tmp_path / "music"
- (library / "an album").mkdir(parents=True)
- (library / "an album" / "track.mp3").write_bytes(_AUDIO_BYTES)
-
- daemon = await _daemon(tmp_path, library, group_id)
- enricher = _CountingEnricher()
- daemon._audio_enricher = enricher
+ group_id = "d" * 32
+ daemon, indexer, roots, _ = await _library(tmp_path, group_id)
try:
- await daemon._roster.set_app_directories(
- group_id, "music", ["music/an album"], set_by="op")
-
- roots = RootSet.build([{"path": str(library), "removable": True}])
- indexer = DirectoryIndexer(
- roots=roots, group_id=group_id,
- sk_node=Ed25519PrivateKey.generate(), gek=generate_gek())
- await indexer.initial_scan()
- await _settled(daemon, indexer)
- assert enricher.spawned == ["track.mp3"]
+ assert _only(indexer).album == "a record"
- # Gone, then back — availability is what reconcile() watches.
roots.roots[0].available = False
await indexer.reconcile()
await _settled(daemon, indexer)
await indexer.reconcile()
await _settled(daemon, indexer)
+ await asyncio.sleep(0.4)
- assert enricher.spawned.count("track.mp3") >= 2, (
+ assert _only(indexer).album == "a record", (
"a drive that fell off and came back left the library with no "
- "metadata until the next daemon restart")
+ "metadata")
finally:
await daemon._media_cache.close()
await daemon._roster.close()
-async def test_videos_and_photos_lost_the_same_fields(tmp_path):
+# ── What genuinely does have to be re-derived ───────────────────────────────
+
+async def test_a_track_moved_while_the_drive_was_away_is_enriched_again(tmp_path):
"""
- Nothing here is specific to Music — Videos and Photos hang off the same
- `new_entries` list in `_broadcast_index_change`, so a replug took their
- durations, titles and thumbnails with it too. Music is simply where it
- shows up loudest: a track with no tags has no album to file it under, so
- the app goes empty rather than merely plain.
+ The counter-case, and the reason the carry-over is keyed on name and path
+ as well as id. `artist`, `album`, `display_title` and `track_no` all fall
+ back to the folder and filename when a file carries no tags, so the same
+ bytes under a new name are not the same metadata. Those are the entries
+ the daemon still re-enriches, off `rescanned_ids`.
"""
- group_id = "c" * 32
+ group_id = "e" * 32
+ enricher = _CountingEnricher()
+ daemon, indexer, _, library = await _library(
+ tmp_path, group_id, enricher=enricher)
+ try:
+ assert enricher.spawned == ["01 first track.mp3"]
+
+ moved = library / "another artist" / "another record"
+ moved.mkdir(parents=True)
+ (library / "an artist" / "a record" / "01 first track.mp3").rename(
+ moved / "01 first track.mp3")
+
+ indexer.eject_root("music")
+ await indexer.plug_root("music")
+ await _settled(daemon, indexer)
+
+ assert enricher.spawned == ["01 first track.mp3"] * 2, (
+ "the file is under a different artist and album now; carrying the "
+ "old ones across would file it under a folder it left")
+ finally:
+ await daemon._media_cache.close()
+ await daemon._roster.close()
+
+
+async def test_videos_and_photos_are_covered_by_the_same_path(tmp_path):
+ """
+ Nothing here is specific to Music — Videos and Photos lost their durations,
+ titles and thumbnails the same way. Music is simply where it shows up
+ loudest: a track with no tags has no album to file it under, so the app
+ goes empty rather than merely plain.
+ """
+ group_id = "f" * 32
library = tmp_path / "media"
(library / "films").mkdir(parents=True)
(library / "films" / "clip.mkv").write_bytes(os.urandom(60 * 1024))
@@ -207,12 +299,20 @@ async def test_videos_and_photos_lost_the_same_fields(tmp_path):
await _settled(daemon, indexer)
assert video.spawned == ["clip.mkv"] and photo.spawned == ["shot.jpg"]
+ by_name = {e.name: e for e in indexer.index.entries}
+ by_name["clip.mkv"].duration = 1234
+ by_name["shot.jpg"].thumb_hash = "a-thumbnail"
+
indexer.eject_root("media")
await indexer.plug_root("media")
await _settled(daemon, indexer)
- assert video.spawned == ["clip.mkv"] * 2, "the film lost its probe"
- assert photo.spawned == ["shot.jpg"] * 2, "the photo lost its thumbnail"
+ back = {e.name: e for e in indexer.index.entries}
+ assert back["clip.mkv"].duration == 1234, "the film lost its probe"
+ assert back["shot.jpg"].thumb_hash == "a-thumbnail", (
+ "the photo lost its thumbnail")
+ assert video.spawned == ["clip.mkv"] and photo.spawned == ["shot.jpg"], (
+ "unchanged files were probed and thumbnailed all over again")
finally:
await daemon._media_cache.close()
await daemon._roster.close()