From 8889e90b8f1629f983c7ed4e44a2badbda525095 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 16 Sep 2026 12:21:04 +0200 Subject: docs: playlists, corrected against what building it needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourteen places the proposal was wrong, listed rather than edited away: the MNP bump was 2.1 and is 3.1; the kind pattern refused the one playlist every account has; confirm() is the house style and prompt() is what throws; submenus expand in place; deflate is 4.5x and the first fixture's 23x was a property of the fixture. Adds the hazard the proposal missed entirely: two wrappers that take two arguments and forward two, so every "add to queue" arrived as a plain play. The acceptance section now says what is checked and what is not — no real daemon round trip, no two-browser convergence, no Firefox. Co-Authored-By: Claude Opus 5 --- docs/MESHBAY_DESIGN.md | 8 +- docs/playlists.md | 412 +++++++++++++++++++++++++++++++++++-------------- 2 files changed, 297 insertions(+), 123 deletions(-) (limited to 'docs') diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index aaa232c..a079e50 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -32,7 +32,7 @@ | **this document** | the architecture, the trust model, and the reason each decision is what it is | | `MESHBAY_NODE_PROTOCOL.md` | the MNP wire format, message by message | | `transfers-v1.md` | the transfer system's failure-mode analysis, kept because a synthesis cannot carry "every way a slot can be lost" | -| `playlists.md` | the playlist design and its interface in full — decided, not built (§9.10) | +| `playlists.md` | the playlist design and its interface in full, with what building it corrected (§9.10) | | `cast-smart-tv.md` | the DLNA/UPnP device backend — designed, not built (§11.4) | | `WINDOWS-PORT.md` | the Windows port's audit and packaging detail (§11.2) | | `PACKAGING-GUIDE.md`, `HTTPS.md`, `MAIL-SERVER.md`, `windows-build.md` | installation and server operations | @@ -2306,11 +2306,11 @@ supports is precise, and the one it must not make matters more: > with file access can already download. What this design controls is what the > *application* computes and surfaces — not what the underlying file contains. -### 9.10 Playlists — designed, not built +### 9.10 Playlists Playlists are the first feature to need **per-account state that spans several -groups on several nodes**, so the shape is settled here even though the code is -not written. Full design: `playlists.md`. +groups on several nodes**. Built 2026-09-16; full design, and every place the +design was wrong before it was built, in `playlists.md`. A playlist belongs to **one account and is never shared with other members.** That scope is what keeps the merge problem small. diff --git a/docs/playlists.md b/docs/playlists.md index ecbbac4..49b1585 100644 --- a/docs/playlists.md +++ b/docs/playlists.md @@ -1,7 +1,13 @@ # MeshBay — Playlists (design) -> Status: **proposal**, not implemented. This was deferred out of the Music -> application's design, which deferred it for the right reason: *"a genuinely +> Status: **built**, 2026-09-16, in five commits. This document was written as +> a proposal and has been corrected against what the code turned out to need — +> §0.2 lists every place the proposal was wrong, because a design document that +> quietly agrees with the implementation afterwards is a document nobody can +> use to check the implementation. +> +> It was deferred out of the Music application's design, which deferred it for +> the right reason: *"a genuinely > new category of per-account node state, not covered by anything E9 already > enumerates — needs its own design pass (ownership, sync across devices, > whether it's node-local or something else)"*. This document is that pass. @@ -53,6 +59,30 @@ fields that actually have to be there. --- +## 0.2 What building it corrected + +Fourteen things. They are listed rather than edited away because the ones that +were *wrong* are more useful than the ones that were right. + +| | The proposal said | Building it found | +|---|---|---| +| 1 | MNP MINOR bump to **2.1** | The wire version was already **3.0**; the bump is **3.1**, and `MNP_MIN_SUPPORTED` does not move because nothing here is required | +| 2 | `kind` is `playlist:` | That refuses `playlist:favorites` — the one playlist every account has. The pattern is `[A-Za-z0-9_-]{1,64}` | +| 3 | The blob is **msgpack**, "same as everything else on MNP" | The node never parses it, so the encoding is a private choice; MNP's codec is private to `transport.js`, a classic script. **JSON**, which is what keeps the merge runnable by `node` with nothing around it | +| 4 | Delete confirms inline, "not `window.confirm()`, which blocks the SPA" | Backwards. `prompt()` **throws** in Electron and is banned by a test; `confirm()` is measured to work and is used in twenty places. A *name* needs a field; a *confirmation* uses `confirm` | +| 5 | Submenus fly out, and are a sheet on a coarse pointer | They **expand in place**, which the account menu's language list already does. No flipping, no hover intent, no separate mobile design. The tracklist level loads when it is expanded | +| 6 | Add all four new files to `STATIC_FILES` / `SPLIT_FILES` | Those lists check hook ordering. The four hookless modules do not belong in them, exactly as `source-merge.js` does not; only `menu.js` and `playlist-menu.js` do | +| 7 | Four modules | **Five**: `playlist-crypto.js` is split out so seal/open can be executed standalone. A crypto layer that cannot be executed is one nobody has checked | +| 8 | "Save the current queue" in the queue panel **and** the toolbar menu | The queue is the player's own state. Mirroring it into a menu in another component would mean lifting that state out of the player for a menu item, so it lives in the queue panel alone | +| 9 | — | **The `op` hazard**, missed entirely. `group-page.js` and `search-page.js` both wrap `onPlayQueue` with a two-argument signature and forwarded two, so every "add to queue" arrived at the player as a plain play. §9.6 | +| 10 | — | **`user_blob_list` is what makes sync quiet.** Comparing against the merged watermark says nothing about what *that node* holds, so the first version pushed every body on every sync. §7.1 | +| 11 | — | **A tombstoned playlist's body must be deleted from each node**, or the account's quota fills with graves. §7.2 | +| 12 | The IndexedDB version is bumped "in the existing `onupgradeneeded`" | And must stay there, in one file. Two modules opening one database at versions of their own is a `VersionError` thrown at whichever runs second. §14.2 | +| 13 | deflate is worth about three | **4.5×** on realistic data — and the first fixture said 23× because it repeated one content hash on every track, which is a property of the fixture. §4.2 | +| 14 | `body_rev` alongside `rev` | Right, and worth saying why: a rename and a track added elsewhere both write n+1 under one counter, and one of two non-overlapping edits is lost. §5.1 | + +--- + ## 1. What this design does not reopen - **Views over the index, never a catalogue** (`MESHBAY_DESIGN.md` §6.5, §9.1). @@ -75,16 +105,17 @@ fields that actually have to be there. --- -## 2. Four problems, and only two of them are hard +## 2. Five problems, and only two of them are hard | | Problem | Where it is solved | |---|---|---| | A | Somewhere durable to put a few hundred KB per account | §3 — several acceptable answers, one clearly best | | B | How to **name** a track that lives in one group on one node, inside a list that spans several | §5.3 — where the silent failures live | | C | Reconciling N copies when nodes are ON and OFF | §6 — the stated fear, well founded *only* for the wrong granularity | -| D | A queue that can be **appended to**, not only replaced | §8 — the player's queue is three `useState`s reset wholesale; that is the one real code change | +| D | A queue that can be **appended to**, not only replaced | §9 — the player's queue is three `useState`s reset wholesale; that is the one real code change | +| E | Getting `op` from the menu to the player **through two wrappers** | §9.2a — not foreseen at all, and the only defect here that shipped in a form nothing on screen would have shown | -The request reads as a storage problem. Storage is the easy quarter of it. +The request reads as a storage problem. Storage is the easy fifth of it. --- @@ -288,11 +319,16 @@ which is the failure this whole document exists to prevent. | limit | value | enforced by | |---|---|---| | manifest (`kind = "playlists"`) | 64 KB sealed | node | -| one body (`kind = "playlist:"`) | 1 MB sealed ≈ 10 000 tracks | node | +| one body (`kind = "playlist:"`) | 1 MB sealed ≈ 17 000 tracks | node | | all rows for one account on one node | 8 MB | node | | tracks per playlist | 10 000 | client, before sealing | | playlists per account | 200 | client, before sealing | +At 4.5× and ~270 bytes a track, a sealed body runs about **60 bytes a track**: +the 1 MB cap is ~17 000 tracks in one playlist, and 8 MB per account is ~140 000 +track references across all of them. Both hold with room to spare, which is the +point of writing the measurement down rather than the guess. + The client limits exist so the ordinary case produces a sentence in the UI rather than an MNP error; the node limits exist because a client is not trusted to hold to them, and this is an unbounded write primitive pointed at @@ -313,6 +349,14 @@ named and then dismissed. ### 5.1 The manifest — `kind = "playlists"` +**JSON, not msgpack**, inside the blob. The node stores bytes and never parses +them, so the encoding is this client's private choice rather than a wire +format — and MNP's own codec is private to `transport.js`, which is a classic +script and cannot be imported. Keeping the payload in reach of `JSON.parse` is +what lets `playlist-merge.js` be executed by `node` with nothing around it, +which is the only real evidence the merge has. After deflate the difference +from msgpack is a few per cent. + Small, always resident, and the only thing every menu needs: ``` @@ -350,9 +394,16 @@ how §6.3's defect ships. ``` Array order *is* playlist order. `rev` here is the body's own, and the -manifest's `body_rev` is a copy of it — a client that sees a manifest naming -`body_rev: 41` while holding body `rev: 39` knows to fetch without fetching to -find out. A body whose manifest entry is `deleted` is dropped locally and +manifest's `body_rev` is a **watermark**: the highest body revision anybody has seen, +merged by taking the maximum and never decreasing, with `count` following +whichever side carries it. A client that sees a manifest naming `body_rev: 41` +while holding body `rev: 39` knows to fetch without fetching to find out. + +The two counters are not redundancy. Rename a playlist on one device while +adding a track to it on another and both write revision n+1; under a single +counter those collide and one of two edits that do not overlap at all is lost. +`rev` carries the name and the tombstone, `body_rev` carries the tracks, and +they move independently. A body whose manifest entry is `deleted` is dropped locally and `user_blob_delete`d on each node as it is reached; the tombstone in the manifest is what survives, not the body. @@ -500,13 +551,30 @@ well as at menu time: the manifest says which bodies are stale (`body_rev`), and a body is pulled when it is opened, loaded, or edited — not on every connection. A user with 40 playlists who only ever plays two never transfers the other 38. -Two additions on top: +As built, that is: **one reconciliation when Music mounts on a connected +group**, and another whenever a track is added to a playlist — both over the +transport that group already has open. And an explicit **"Sync now"** in the +playlist menu. **On sign-in, nothing**: the local copy is authoritative and +complete (§6.4), and the first group opened reconciles. -- **An explicit "Sync now"** in the playlist menu, which does the Search-style - sweep and reports which nodes it could not reach — the same honest reporting - `fetchAllIndexes` already does. -- **On sign-in, nothing.** The local copy is authoritative and complete (§6.4); - the first group opened will reconcile. +### 7.1 Ask the node what it holds, never guess + +`user_blob_list` returns kinds and revisions, no payloads, in one message. Sync +compares against **that**, not against the merged watermark. + +The difference is not an optimisation. `body_rev` in the manifest is the highest +revision *anybody* has seen; it says nothing about whether this particular node +has it. The first version of `syncWith` compared against it and so could not +tell "this node is behind" from "this node is up to date" — and pushed every +body on every sync, over a connection it had borrowed from something else. + +### 7.2 A tombstone has a body to reclaim + +Deleting a playlist tombstones its manifest entry, and the entry is what has to +survive. The **body** does not: it is `user_blob_delete`d from each node as that +node is reached. Without that step the body of every playlist ever deleted stays +on every node for ever, and the account's 8 MB quota fills up with graves — +which the node is entitled to refuse the next real write for. A device that only ever opens one group therefore only ever converges with one node. That is correct and not a defect: convergence is eventual, and the copy @@ -518,7 +586,7 @@ was reachable, which is usually A's, and spreads from there. ## 8. Protocol and node-side implementation -### 8.1 MNP — additive, MINOR bump (2.1) +### 8.1 MNP — additive, MINOR bump (3.1) ``` user_blob_store { kind, rev, blob_enc } client → node @@ -537,8 +605,22 @@ tells the node nothing it does not already know from its own table. Modelled on `KEYPAIR_BUNDLE_*` in every respect, including that the node stores and returns an opaque byte string. `kind` is validated against a **pattern** — -`playlists` or `playlist:` — so the table does not become an arbitrary -key/value store for whatever a client feels like writing. +`playlists`, or `playlist:` and one to sixty-four of `[A-Za-z0-9_-]` — so the +table does not become an arbitrary key/value store for whatever a client feels +like writing. + +The character class is deliberately wider than a UUID, and that is a correction: +the reserved id is the word `favorites` (§5.1), so a hex-only pattern refuses +the one playlist every account has. It stays narrow enough to carry no structure +of its own — no `/`, no `.`, no second `:` — so a kind can never be read as a +path or as anything but one name in one namespace. + +**The bump is 3.1, not 2.1.** The proposal read the version off an older +document; the wire version was already 3.0. `MNP_MIN_SUPPORTED` stays at 3.0, +because nothing here is *required*: a 3.0 node answers "unknown message type" +and the client stores its playlists on the next node it reaches, keeping its own +copy meanwhile (§6.4). That is the difference from the 3.0 bump, where the +requirement rather than the messages is what made it MAJOR. Every reply carries `req_id` through the ordinary `_send` path. This is not optional and does not need re-arguing: `CLAUDE.md` records at length what @@ -622,6 +704,33 @@ onPlayQueue(tracks, startIndex, source, op) // op: 'replace' | 'next' | 'appen move, the `