diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-02 15:23:30 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-02 15:23:30 +0200 |
| commit | 313b72f15e8788ba3abcd3e44b5f7785fbc779fe (patch) | |
| tree | 1c8e9b024d78384da950730b37b8d41c2789e010 /packages/meshbay-hub/tests | |
| parent | 15d3eec914cf5e474e66f615c9fbaf602eebe575 (diff) | |
| download | meshbay-313b72f15e8788ba3abcd3e44b5f7785fbc779fe.tar.gz | |
fix(hub): one entry per file in the Search view's Videos grid
A library shared by two groups arrived in the cross-group Search view as
two entries per file: every film was two poster cards, every episode was
listed twice in the season list under the synopsis. Inside one group
this cannot happen — GroupIndex is keyed by blake3 — so the duplication
was the Search page's own, from concatenating N independently keyed
indexes.
source-merge.js folds entries on the content hash and resolves one
source per *unit* (a film, a whole show), so a season does not scatter
across two nodes. A group hosted by the reader's own node wins; failing
that the pick is a hash of the unit key and the reader's id, stable
across renders and reloads — a source that changed mid-stream would tear
down the connection under a film that is playing — and spread across
readers and units.
The units come from video-app.js's own groupVideoEntries rather than a
second copy of its keys here. Only the Videos view is wired up so far;
Music, Photos, failover and the "N sources" badge are phases 5-8 of
docs/refactoring-search.md.
Every test was checked against the fix removed. That is how the first
version of "a unit's files share its source" turned out to prove
nothing: with every episode in every group, per-file and per-unit
picking give the same answer, so it passed against a per-file
implementation. It now uses a unit whose files have unequal sources.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AbwJDbNTkiRUh7HTWEoyss
Diffstat (limited to 'packages/meshbay-hub/tests')
| -rw-r--r-- | packages/meshbay-hub/tests/test_search_source_merge.py | 340 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_search_video_merge.py | 185 |
2 files changed, 525 insertions, 0 deletions
diff --git a/packages/meshbay-hub/tests/test_search_source_merge.py b/packages/meshbay-hub/tests/test_search_source_merge.py new file mode 100644 index 0000000..078c38d --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_source_merge.py @@ -0,0 +1,340 @@ +""" +One file, one entry: merging the same content announced by several groups. + +Reported live: a node hosting two groups that were given the same video +directory — the point of two groups being that different people are invited to +different libraries, and one library may be in several of them. The Search +view, which concatenates every group's index into one list, then showed every +film twice, every episode twice inside a show, every track twice inside an +album. Inside one group this cannot happen: `GroupIndex` is keyed by blake3, so +the same bytes at two paths are already one entry. The duplication is the +Search page's own. + +`source-merge.js` merges on the content hash and resolves one source per +*unit* — a film, a whole show, a whole album — rather than per file, so a +season's episodes do not scatter across two nodes. This holds the rules that +decide which source that is, and what survives the merge. + +The whole module is executed here rather than a regex-extracted function of it: +it has no imports precisely so that it can be, and a copy of the picking rule +in a test would keep agreeing with the original right up until one of them +changed. + +See docs/refactoring-search.md. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +SRC = STATIC / "source-merge.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not SRC.exists(), + reason="node or the SPA sources are not available") + +IMPORT = re.compile(r"^\s*import\b", re.M) +EXPORT = re.compile(r"^export \{[^}]*\};?\s*$", re.M) + + +@pytest.fixture(scope="module") +def module_source(): + text = SRC.read_text() + assert not IMPORT.search(text), ( + "source-merge.js has gained an import. It is executed standalone here, " + "and the merge is untested from the moment it cannot be — keep the " + "module free of imports, or this test needs a bundler") + stripped, n = EXPORT.subn("", text) + assert n == 1, ( + "source-merge.js no longer ends in a single export statement — the " + "test can no longer strip it to run the module") + return stripped + + +def _run(tmp_path, module_source, body): + script = tmp_path / "case.js" + script.write_text(f"{module_source}\n{body}\n") + out = subprocess.run( + ["node", str(script)], capture_output=True, text=True, timeout=30) + assert out.returncode == 0, out.stderr + return json.loads(out.stdout) + + +def _entry(file_id, group, **kw): + e = { + "id": file_id, + "name": f"{file_id}.mkv", + "type": "video", + "groupId": group, + "groupName": group.upper(), + "groupOwner": "someone", + "_tRef": f"t:{group}", + } + e.update(kw) + return e + + +def _merge(tmp_path, module_source, units, salt="u1", local=(), down=()): + body = f""" + const units = {json.dumps(units)}; + const local = new Set({json.dumps(list(local))}); + const down = new Set({json.dumps(list(down))}); + const merged = mergeUnitEntries(units, {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + isDown: (g) => down.has(g), + }}); + console.log(JSON.stringify(merged)); + """ + return _run(tmp_path, module_source, body) + + +# ── the reported case ──────────────────────────────────────────────────────── + +def test_one_file_two_groups_becomes_one_entry(tmp_path, module_source): + """The whole bug: a film shared by two groups was two poster cards.""" + units = [ + {"key": "movie:aaa", "entries": [_entry("aaa", "demo35")]}, + {"key": "movie:aaa", "entries": [_entry("aaa", "media")]}, + ] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert merged[0]["id"] == "aaa" + assert [s["groupId"] for s in merged[0]["_sources"]] == ["demo35", "media"] + + +def test_a_show_is_one_list_of_episodes(tmp_path, module_source): + """Two seasons' worth of episodes announced twice, one list out.""" + eps = [f"e{n}" for n in range(6)] + units = [{ + "key": "show:Some Saga", + "entries": [_entry(i, g) for g in ("demo35", "media") for i in eps], + }] + merged = _merge(tmp_path, module_source, units) + assert sorted(e["id"] for e in merged) == sorted(eps) + + +def test_a_lone_source_is_left_alone(tmp_path, module_source): + """The overwhelmingly common case must come through untouched.""" + units = [{"key": "movie:aaa", "entries": [_entry("aaa", "solo")]}] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert merged[0]["groupId"] == "solo" + assert merged[0]["_tRef"] == "t:solo" + assert len(merged[0]["_sources"]) == 1 + + +# ── which source ───────────────────────────────────────────────────────────── + +def test_the_local_node_always_wins(tmp_path, module_source): + """ + Whatever the salt, a group on the reader's own node is the source. Every + salt is tried rather than one: a rule that only holds for the salt the test + happened to pick is not the rule. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "remote"), _entry("aaa", "mine")]}] + for salt in [f"user-{n}" for n in range(20)]: + merged = _merge(tmp_path, module_source, units, salt=salt, local=["mine"]) + assert merged[0]["groupId"] == "mine", f"salt {salt} escaped the local rule" + + +def test_the_pick_is_stable_and_spread(tmp_path, module_source): + """ + Stable for one reader — a source that changed between renders would tear + down the connection under a film that is playing — and different across + readers, which is what "random" was for. + """ + groups = [f"g{n}" for n in range(8)] + units = [{"key": "movie:aaa", "entries": [_entry("aaa", g) for g in groups]}] + + first = _merge(tmp_path, module_source, units, salt="u1")[0]["groupId"] + again = _merge(tmp_path, module_source, units, salt="u1")[0]["groupId"] + assert first == again + + picks = {_merge(tmp_path, module_source, units, salt=f"u{n}")[0]["groupId"] + for n in range(40)} + assert len(picks) > 1, "every reader lands on the same source — nothing is spread" + + +def test_one_reader_is_spread_across_sources(tmp_path, module_source): + """ + The spread is across units as well as across readers. Hashing the salt + alone would be just as stable and just as fair between readers, and would + point one reader's entire library at one node. + """ + groups = ("aa", "bb", "cc", "dd") + units = [{"key": f"movie:{n}", "entries": [_entry(f"f{n}", g) for g in groups]} + for n in range(40)] + merged = _merge(tmp_path, module_source, units, salt="u1") + assert len({e["groupId"] for e in merged}) > 1 + + +def test_source_order_does_not_decide(tmp_path, module_source): + """ + The pick sorts by group id first. Without that it would follow whichever + group the index fetch happened to answer first, which is a race. + """ + groups = [_entry("aaa", g) for g in ("zulu", "alpha", "mike")] + fwd = _merge(tmp_path, module_source, [{"key": "m", "entries": groups}]) + rev = _merge(tmp_path, module_source, [{"key": "m", "entries": groups[::-1]}]) + assert fwd[0]["groupId"] == rev[0]["groupId"] + + +def test_every_file_of_a_unit_shares_its_source(tmp_path, module_source): + """ + The reason the source is picked per unit at all: a season split across two + nodes would open two connections and two metadata lookups for one show. + + The episodes deliberately do NOT all have the same sources. With every + episode in every group, picking per file and picking per unit give the same + answer — the same key over the same set — and a test built that way passes + against a per-file implementation, which is how the first version of this + one got written. `e3` is missing from `cc`, so a per-file pick resolves it + over `{aa, bb}` while its siblings resolve over `{aa, bb, cc}`, and the two + rules come apart. + """ + full = ("aa", "bb", "cc") + units = [{ + "key": "show:Some Saga", + "entries": ([_entry(i, g) for g in full for i in ("e1", "e2")] + + [_entry("e3", g) for g in ("aa", "bb")]), + }] + exercised = 0 + for salt in [f"u{n}" for n in range(40)]: + by_id = {e["id"]: e for e in + _merge(tmp_path, module_source, units, salt=salt)} + assert by_id["e1"]["groupId"] == by_id["e2"]["groupId"] + chosen = by_id["e1"]["groupId"] + if chosen == "cc": + continue # e3 genuinely does not have it; it falls back + exercised += 1 + assert by_id["e3"]["groupId"] == chosen, ( + f"salt {salt}: e3 has {chosen} and went to " + f"{by_id['e3']['groupId']} instead — the source is being picked " + "per file, not per unit") + assert exercised > 5, ( + "no salt put the unit on a source e3 also has — this test proved " + "nothing about per-unit picking") + + +def test_a_file_the_unit_source_lacks_falls_back_together(tmp_path, module_source): + """ + An episode only one group holds still plays — and when several episodes are + missing from the chosen source they all land on the same fallback rather + than one node apiece. + """ + shared = [_entry(i, g) for g in ("aa", "bb") for i in ("e1", "e2")] + # e8/e9 exist in bb and cc only, whichever of the three the unit picked + extra = [_entry(i, g) for g in ("bb", "cc") for i in ("e8", "e9")] + merged = _merge(tmp_path, module_source, + [{"key": "show:Some Saga", "entries": shared + extra}]) + by_id = {e["id"]: e for e in merged} + assert sorted(by_id) == ["e1", "e2", "e8", "e9"] + assert by_id["e8"]["groupId"] == by_id["e9"]["groupId"] + assert by_id["e8"]["groupId"] in ("bb", "cc") + + +# ── failover ───────────────────────────────────────────────────────────────── + +def test_a_down_source_is_skipped(tmp_path, module_source): + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", g) for g in ("aa", "bb", "cc")]}] + for salt in [f"u{n}" for n in range(20)]: + merged = _merge(tmp_path, module_source, units, salt=salt, down=["aa", "bb"]) + assert merged[0]["groupId"] == "cc" + + +def test_a_down_local_source_yields_to_a_live_remote(tmp_path, module_source): + """ + The local preference is a preference, not an override. A local node that is + not answering must not hold a film hostage while another group serves it. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "mine"), _entry("aaa", "remote")]}] + merged = _merge(tmp_path, module_source, units, local=["mine"], down=["mine"]) + assert merged[0]["groupId"] == "remote" + + +def test_everything_down_still_yields_an_entry(tmp_path, module_source): + """A tile that fails to load beats a film that vanished from the grid.""" + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", g) for g in ("aa", "bb")]}] + merged = _merge(tmp_path, module_source, units, down=["aa", "bb"]) + assert len(merged) == 1 + assert merged[0]["groupId"] in ("aa", "bb") + assert len(merged[0]["_sources"]) == 2, "a down source is still a source" + + +# ── what survives the merge ────────────────────────────────────────────────── + +def test_fields_come_from_the_chosen_source_only(tmp_path, module_source): + """ + No field is back-filled from another source. A thumb_hash only one node + computed is only fetchable over *that* node's connection, so borrowing it + would produce a poster request the chosen transport cannot answer. + """ + units = [{"key": "movie:aaa", "entries": [ + _entry("aaa", "mine", thumb_hash=None, display_title="a"), + _entry("aaa", "remote", thumb_hash="deadbeef", display_title="b"), + ]}] + merged = _merge(tmp_path, module_source, units, local=["mine"]) + assert merged[0]["groupId"] == "mine" + assert merged[0]["thumb_hash"] is None + assert merged[0]["display_title"] == "a" + assert merged[0]["_tRef"] == "t:mine" + + +def test_units_are_kept_apart(tmp_path, module_source): + """ + Merging is scoped to a unit. Two albums that happen to hold the same photo + are two albums, and it belongs in both. + """ + units = [ + {"key": "album:trip", "entries": [_entry("p1", "aa"), _entry("p1", "bb")]}, + {"key": "album:party", "entries": [_entry("p1", "aa")]}, + ] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 2 + + +def test_one_group_announcing_a_file_twice_counts_once(tmp_path, module_source): + """ + Cannot happen through a well-behaved index (GroupIndex is keyed by id), so + a duplicate here is a node saying something odd. It must not inflate the + source count the badge shows. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "aa"), _entry("aaa", "aa")]}] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert len(merged[0]["_sources"]) == 1 + + +def test_empty_input(tmp_path, module_source): + assert _merge(tmp_path, module_source, []) == [] + + +# ── the badge ──────────────────────────────────────────────────────────────── + +def test_source_label(tmp_path, module_source): + body = """ + const one = { groupId: 'g1', groupName: 'G1', + _sources: [{ groupId: 'g1', groupName: 'G1' }] }; + const many = { groupId: 'g1', groupName: 'G1', + _sources: [{ groupId: 'g1' }, { groupId: 'g2' }] }; + const bare = { }; + console.log(JSON.stringify( + [one, many, bare].map(sourceLabel))); + """ + one, many, bare = _run(tmp_path, module_source, body) + assert one == {"count": 1, "name": "G1", "groupId": "g1"} + # Which group was picked is deliberately not shown once there are several. + assert many == {"count": 2, "name": "", "groupId": None} + # The single-group Group page, where entries carry no sources at all. + assert bare == {"count": 1, "name": "", "groupId": None} diff --git a/packages/meshbay-hub/tests/test_search_video_merge.py b/packages/meshbay-hub/tests/test_search_video_merge.py new file mode 100644 index 0000000..e9ab258 --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_video_merge.py @@ -0,0 +1,185 @@ +""" +The reported symptom, end to end: one library shared by two groups. + +`test_search_source_merge.py` holds the merging rules in isolation. This holds +the thing an operator actually saw — a node hosting two groups that were given +the same video directory, and a Search view showing every film as two poster +cards and every episode twice inside a show. + +Three pieces have to agree for that to come out right, and each lives in a +different file: + + * `groupVideoEntries` (video-app.js) turns entries into films and shows; + * `videoUnits` (search-page.js) turns those into merge units; + * `mergeUnitEntries` (source-merge.js) folds them on the content hash. + +All three are read out of their real sources here rather than restated. The +pipeline is assembled the way `search-page.js` assembles it, and then the +result is passed through `groupVideoEntries` a second time — which is what +`VideoApp` does with it — so what this counts is what the grid renders. + +See docs/refactoring-search.md. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +VIDEO_APP = STATIC / "video-app.js" +SEARCH_PAGE = STATIC / "search-page.js" +MERGE = STATIC / "source-merge.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not MERGE.exists(), + reason="node or the SPA sources are not available") + +EXPORT = re.compile(r"^export \{[^}]*\};?\s*$", re.M) + + +def _block(path, header): + """One top-level `function name(...) {` ... `}` read out of a module.""" + src = path.read_text() + m = re.search(r"^" + re.escape(header) + r".*?^\}", src, re.M | re.S) + assert m, ( + f"{header} is no longer where this test reads it from in {path.name} — " + "the Search view's de-duplication is untested until this is fixed") + return m.group(0) + + +@pytest.fixture(scope="module") +def pipeline(): + root = re.search(r"^const SEARCH_VIDEO_ROOT = .*?;$", SEARCH_PAGE.read_text(), re.M) + assert root, "SEARCH_VIDEO_ROOT moved — videoUnits cannot be lifted" + return "\n".join([ + EXPORT.sub("", MERGE.read_text()), + _block(VIDEO_APP, "function underVideoRoot(entry, videoRoot) {"), + _block(VIDEO_APP, "function buildSeasons(episodes) {"), + _block(VIDEO_APP, "function groupVideoEntries(entries, videoRoot) {"), + root.group(0), + _block(SEARCH_PAGE, "function videoUnits(entries) {"), + ]) + + +def _grid(tmp_path, pipeline, entries, salt="reader", local=()): + """What the poster grid ends up with, after the merge and VideoApp's own + regrouping.""" + body = f""" + const raw = {json.dumps(entries)}; + const local = new Set({json.dumps(list(local))}); + const merged = mergeUnitEntries(videoUnits(raw), {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + }}); + const {{ movies, shows }} = groupVideoEntries(merged, SEARCH_VIDEO_ROOT); + console.log(JSON.stringify({{ + movies: movies.map((e) => ({{ + id: e.id, title: e.display_title || e.name, groupId: e.groupId, + sources: e._sources.length, + }})), + shows: shows.map((s) => ({{ + title: s.title, + groups: [...new Set(s.episodes.map((e) => e.groupId))].sort(), + seasons: s.seasons.map((x) => ({{ + season: x.season, + episodes: x.episodes.map((e) => `S${{e.season}}E${{e.episode}}`), + }})), + }})), + }})); + """ + script = tmp_path / "case.js" + script.write_text(f"{pipeline}\n{body}\n") + out = subprocess.run( + ["node", str(script)], capture_output=True, text=True, timeout=30) + assert out.returncode == 0, out.stderr + return json.loads(out.stdout) + + +# The shape of one shared library: a film, and a two-season show. Invented +# titles — the real one this was found against is nobody's business here. +def _library(group): + """`path` is already prefixed the way search-page.js prefixes it.""" + def entry(file_id, name, **kw): + return { + "id": file_id, "name": name, "type": "video", + "path": "__search__/shows", "size": 1, + "groupId": group, "groupName": group.upper(), "groupOwner": "someone", + "_tRef": f"t:{group}", "_gRef": f"g:{group}", "_connGen": 1, + **kw, + } + files = [entry("film1", "a-film.mkv", display_title="Some Film")] + for season in (1, 2): + for ep in (1, 2, 3): + files.append(entry( + f"s{season}e{ep}", f"show.s0{season}e0{ep}.mkv", + display_title="Some Saga", season=season, episode=ep)) + return files + + +def test_a_shared_library_is_listed_once(tmp_path, pipeline): + """ + The bug as reported: two groups, one directory, everything twice. + """ + both = _library("demo35") + _library("media") + grid = _grid(tmp_path, pipeline, both) + + assert [m["title"] for m in grid["movies"]] == ["Some Film"] + assert grid["movies"][0]["sources"] == 2 + + assert len(grid["shows"]) == 1 + show = grid["shows"][0] + assert [s["season"] for s in show["seasons"]] == [1, 2] + for season in show["seasons"]: + assert season["episodes"] == [ + f"S{season['season']}E{n}" for n in (1, 2, 3)], ( + "an episode is listed more than once — this is the reported bug, " + "in the season list under the synopsis") + + +def test_a_show_streams_from_one_source(tmp_path, pipeline): + """A season split across two nodes would open two connections and two + metadata lookups for one show.""" + both = _library("demo35") + _library("media") + show = _grid(tmp_path, pipeline, both)["shows"][0] + assert len(show["groups"]) == 1 + + +def test_the_operators_own_node_serves_it(tmp_path, pipeline): + """Both groups are on the operator's node in the reported case; when only + one is, that one is the source.""" + both = _library("remote") + _library("mine") + grid = _grid(tmp_path, pipeline, both, local=["mine"]) + assert grid["movies"][0]["groupId"] == "mine" + assert grid["shows"][0]["groups"] == ["mine"] + + +def test_one_group_is_unchanged(tmp_path, pipeline): + """The overwhelmingly common case: nothing to merge, nothing different.""" + grid = _grid(tmp_path, pipeline, _library("solo")) + assert [m["title"] for m in grid["movies"]] == ["Some Film"] + assert grid["movies"][0]["sources"] == 1 + assert grid["movies"][0]["groupId"] == "solo" + show = grid["shows"][0] + assert show["groups"] == ["solo"] + assert sum(len(s["episodes"]) for s in show["seasons"]) == 6 + + +def test_an_episode_only_one_group_has_is_kept(tmp_path, pipeline): + """ + Merging must never subtract. A group holding one extra episode contributes + it, whichever source the show settled on. + """ + extra = _library("media") + extra.append({ + "id": "s2e4", "name": "show.s02e04.mkv", "type": "video", + "path": "__search__/shows", "size": 1, + "groupId": "media", "groupName": "MEDIA", "groupOwner": "someone", + "display_title": "Some Saga", "season": 2, "episode": 4, + }) + grid = _grid(tmp_path, pipeline, _library("demo35") + extra) + season2 = [s for s in grid["shows"][0]["seasons"] if s["season"] == 2][0] + assert season2["episodes"] == ["S2E1", "S2E2", "S2E3", "S2E4"] |