aboutsummaryrefslogtreecommitdiffstats
path: root/docs
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-02 16:21:48 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-02 16:21:48 +0200
commit10f8266e7152d7dc38dbfe2449327829bf020ad1 (patch)
tree5e72ac45b2e79008812663e251a4fcab62dc0650 /docs
parent313b72f15e8788ba3abcd3e44b5f7785fbc779fe (diff)
downloadmeshbay-10f8266e7152d7dc38dbfe2449327829bf020ad1.tar.gz
fix(hub): merge duplicate sources in Search's Music and Photos too
Phases 5-8 of docs/refactoring-search.md, extending the Videos merge outward. A library shared by two groups now lists each track once inside an album and each photo once inside a photo album, and a card served by several groups says "N sources" instead of naming one of them. Units come from each application's own grouping, never a copy of its keys. For Music that meant exporting foldKey: groupMusicEntries folds case to group but keeps the first-seen spelling to display, and which group is seen first is whichever index arrived first — so keying a unit on the display strings would let the chosen source change between page loads. A group whose connection fails is marked down and stops being chosen, so a unit fails over to another group that has the file. Eviction is not a failure. Every source being down still yields an entry: a tile that fails to load beats a film that vanished from the grid. sourceLabel now takes the whole unit rather than one entry. A show's poster entry is picked for its thumbnail, so a show in two groups whose cover episode sits in only one of them would have claimed a single source. SourceTag lives in group-name.js — source-merge.js must keep importing nothing (its test executes it standalone), and a copy in each of the three apps is three chances to disagree. test_search_files_unmerged.py holds the one thing that must not change: the Files explorer is not merged, because there each group is a folder and merging would remove a file from one of them. It also asserts the other three lists are merged, or deleting the merge outright would leave it passing and saying nothing. One plan item was dropped as wrong rather than built: the Music queue in onPreview needed no change. It filters by groupId and is reachable only from FilesPanel, which is not merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AbwJDbNTkiRUh7HTWEoyss
Diffstat (limited to 'docs')
-rw-r--r--docs/refactoring-search.md66
1 files changed, 43 insertions, 23 deletions
diff --git a/docs/refactoring-search.md b/docs/refactoring-search.md
index f23e1ef..2210397 100644
--- a/docs/refactoring-search.md
+++ b/docs/refactoring-search.md
@@ -1,9 +1,10 @@
# Refactor: one file, one entry — merging duplicate sources in the Search view
-> Status: **phases 1–4 built** (2026-09-02), 5–9 planned. Branch
-> `feat/search-source-merge`. The Videos view is merged and the reported bug is
-> gone under test; Music, Photos, failover and the badge are still to come, so
-> a merged film still shows its chosen group's name rather than `N sources`.
+> Status: **phases 1–8 built** (2026-09-02), 9 (the doc pass) outstanding.
+> Branch `feat/search-source-merge`. Videos, Music and Photos are merged,
+> failover is in, and a card with several sources says `N sources` instead of a
+> group name. Confirmed live for Videos on 2026-09-02: one card per film,
+> playback and download work from a merged entry.
> Scope: the cross-group Search page (`static/search-page.js`) and the three
> media applications it reuses (Videos, Music, Photos). The Files **explorer**
> inside Search is explicitly out of scope and must not change.
@@ -179,14 +180,16 @@ file and running them under node, the idiom `test_video_default_season.py`
already uses.
```
-mergeUnitEntries(units, opts) → { entries, unitSourceByKey }
-pickSource(sources, unitKey, salt, isDown) → source
-sourceLabel(entry) → { text, groupId | null }
+mergeUnitEntries(units, opts) → merged entries
+pickSource(sources, unitKey, opts) → source | null
+sourceLabel(entries | entry) → { count, name, groupId }
```
- `units` is a list of `{ key, entries }`. **The unit lists are produced by the
applications' own grouping functions**, never by a second copy of them (§5.2).
- `opts` carries `salt` (the user id), `isLocal(groupId)` and `isDown(groupId)`.
+- `sourceLabel` returns a shape, not a string: the module imports nothing (its
+ test executes it standalone), so it holds no reference to `i18n.js`.
- Each output entry is one merged `IndexEntry` plus `_sources` (every group that
has it, sorted by group id) and the resolved `groupId` / `_tRef` / `_gRef` /
`_connGen` of its effective source.
@@ -275,14 +278,20 @@ one source behaves exactly as it does today.
### 5.5 The badge
-One helper, `sourceLabel(entry)`, four call sites:
+One component, `SourceTag`, four call sites:
-| File | Line today | Today | After |
+| File | Today | After | Counts over |
|---|---|---|---|
-| `video-app.js` | 405 | `repEntry.groupName` | group name, or `N sources` |
-| `video-app.js` | 986 | group-name link badge | same, non-clickable when `N > 1` |
-| `music-app.js` | 267 | `repTrack.groupName` | same rule |
-| `photos-app.js` | 96 | `cover.groupName` | same rule |
+| `video-app.js` `PosterCard` | `repEntry.groupName` | group name, or `N sources` | the movie, or the show's episodes |
+| `video-app.js` `FlatMovieRow` | group-name link badge | same, non-clickable when `N > 1` | the one entry |
+| `music-app.js` `AlbumCard` | `repTrack.groupName` | same rule | the album's tracks |
+| `photos-app.js` `AlbumCard` | `cover.groupName` | same rule | the album's photos |
+
+**`SourceTag` lives in `group-name.js`**, not in `source-merge.js` (which must
+keep importing nothing) and not in each of the three apps (a copy apiece is
+three chances to disagree about what a merged card says). It renders a `div` by
+default — the three card badges rely on `text-overflow: ellipsis`, which does
+nothing on an inline box — and the flat row's inline pill on `link`.
`files-app.js:446` is **not** in this list: the Files explorer is not merged and
its group column keeps naming exactly one group.
@@ -309,10 +318,14 @@ be easy to ship and hard to notice.
transport, set from `connectGroup(entry.groupId)`. With one `groupId` per
merged entry this is unchanged by construction — but a merged entry with a
*null* `groupId` would silently break both, so the merge must never emit one.
-4. **The music queue.** `onPreview`'s audio branch builds siblings by
- `e.path === origPath && e.groupId === groupId` over `allEntries`, which is the
- *un-merged* list. It must be rebuilt over the merged music entries, or a queue
- will contain each track twice — the exact bug, one layer down.
+4. **The music queue.** ~~`onPreview`'s audio branch must be rebuilt over the
+ merged entries or a queue will contain each track twice.~~ **Wrong, checked
+ 2026-09-02 and left alone.** That branch builds siblings over the un-merged
+ `allEntries` but filters `e.groupId === groupId`, so a queue only ever holds
+ one group's copies. It is also reachable only from `FilesPanel`, which is not
+ merged — the Music view never goes through `onPreview`, it calls `onPlayQueue`
+ with its own (merged) tracks. Both facts had to be false for the bug to
+ exist; neither is.
5. **The connection pool's eviction path.** `_onEvict` drops `groupConns` and
bumps `connectionGen`. Eviction is not failure and must not mark a group down.
6. **TMDB / MusicBrainz overrides.** An operator's "Fix match" and "Rematch" go
@@ -335,15 +348,21 @@ Each phase is independently testable and leaves the tree working.
| 2 ✅ | `source-merge.js` — `pickSource`, `mergeUnitEntries`, `sourceLabel`. No caller yet | new file |
| 3 ✅ | Tests for phase 2, read out of the source | `test_search_source_merge.py`, `test_search_video_merge.py` |
| 4 ✅ | Wire the Videos list through the merge | `search-page.js` |
-| 5 | Same for Music, including the `onPreview` queue (§6.4) | `search-page.js` |
-| 6 | Same for Photos | `search-page.js` |
-| 7 | `downGroups` and failover | `search-page.js` |
-| 8 | `sourceLabel` at the four display sites; `search.n_sources` in ten catalogues | `video-app.js`, `music-app.js`, `photos-app.js`, `locales/*.js` |
+| 5 ✅ | Same for Music (`foldKey` exported — an album's display strings are the first-seen spelling, so keying a unit on them would let the source change between page loads) | `search-page.js`, `music-app.js` |
+| 6 ✅ | Same for Photos | `search-page.js` |
+| 7 ✅ | `downGroups` and failover | `search-page.js` |
+| 8 ✅ | `SourceTag` at the four display sites; `search.n_sources` in ten catalogues | `group-name.js`, `video-app.js`, `music-app.js`, `photos-app.js`, `locales/*.js` |
| 9 | Docs: `mediacenter.md` §10, `musicbay.md`, `photos.md`, `apps.md` if the props contract moves | `docs/` |
-Phase 4 alone is enough to confirm the reported bug is gone; phases 5–8 are the
+Phase 4 alone was enough to confirm the reported bug is gone; phases 5–8 are the
same mechanism applied outward.
+**One thing phase 8 changed about the design.** `sourceLabel` was specified to
+take one entry. A card stands for a *unit*, and the entry it is drawn from is
+picked for its thumbnail — `episodes.find((e) => e.thumb_hash)` — so a show in
+two groups whose cover episode sits in only one of them would have said one
+source. It takes the whole unit now, and unions.
+
---
## 8. Tests
@@ -358,8 +377,9 @@ value from the real source rather than restating a constant.
| `test_search_source_merge.py` ✅ | The whole of `source-merge.js` executed standalone — it has no imports precisely so that it can be, and the test refuses a build where it gains one. A file in two groups yields one entry with two sources; a local source always wins, over every salt; the pick is stable across calls, varies with the salt, and spreads one reader across units; source order does not decide it; a unit's files share the unit's source; a file the unit's source lacks falls back with its siblings; a down group is skipped, a down *local* group yields to a live remote, all-down still returns an entry; no field is back-filled from another source |
| `test_search_video_merge.py` ✅ | The reported symptom end to end. `groupVideoEntries` + `buildSeasons` (video-app.js) and `videoUnits` (search-page.js) are lifted from their real sources, the pipeline is assembled as the page assembles it, and the result is re-grouped the way `VideoApp` re-groups it — so what is counted is what the grid renders. Two groups sharing one library give one film card and one show whose seasons hold three episodes, not six; one group is unchanged; an episode only one group has survives |
| extend `test_locales.py` | already fails on a key present in `en.js` and missing elsewhere — no change needed, listed so the ten-catalogue edit is not forgotten |
-| `test_search_files_unmerged.py` | reads `search-page.js` and refuses a build where `fileEntries` is fed through the merge — the one guarantee §6.1 makes, and the one a later refactor is most likely to break by tidying the four lists into one |
+| `test_search_files_unmerged.py` ✅ | reads `search-page.js` and refuses a build where `fileEntries` is fed through the merge — the one guarantee §6.1 makes, and the one a later refactor is most likely to break by tidying the four lists into one. It also asserts the other three lists *are* merged, or deleting the merge outright would leave it passing and saying nothing |
| extend `test_transport_contracts.py` | the existing "declared vs. called setters" check covers the new state in `search-page.js` for free |
+| `test_search_media_merge.py` ✅ | the same end-to-end shape for Music (an album's tracks listed once; a differently-cased tag not splitting the unit; a bonus track only one group has; an untagged track kept as its own unit) and Photos (one album, each photo once; two differently-named albums each keeping the photo) |
**Every one of these was checked against the fix removed**, which is where the
first version of "a unit's files share its source" turned out to prove nothing: