diff --git a/docs/product/bank-package.md b/docs/product/bank-package.md index 4a0e1b2..c565667 100644 --- a/docs/product/bank-package.md +++ b/docs/product/bank-package.md @@ -8,10 +8,13 @@ import as a single-file package**. The tickable spec lives in `docs/PLAN.md` and the reasoning that produced it, the failure-mode table, and what a package deliberately does not carry. -Status: framed by product-designer (2026-08-02). **Three forks are open and are -Daniel's** — Ε-F1 (container format), Ε-F2 (import target), Ε-F3 (import under a -degraded ledger). Everything else below is a product-designer call with its -reasoning stated; contradict it in review with an argument, not a preference. +Status: framed by product-designer (2026-08-02); **all three [Daniel]-class forks +RULED the same day** — Ε-F1 **proprietary container** (`RSBK`), Ε-F2 **import always +lands as a new bank**, with an automatic suffix on a name collision, Ε-F3 **refuse an +import while the tracking ledger is degraded**. See §"Rulings" for the index and the +recorded rationale; each is specified in place in the section that owns it. Nothing in +this doc is open. Everything else below is a product-designer call with its reasoning +stated; contradict it in review with an argument, not a preference. --- @@ -76,63 +79,50 @@ sidecar JSON beside it." Rejected, for four reasons, in descending order of weig folder looks exactly like a complete one. The counter-argument for the folder is real and should be recorded: a folder is -inspectable with no tooling. That argument is answered — partially — by fork Ε-F1 -below, not by abandoning the single file. +inspectable with no tooling. The container ruling below does **not** buy that back — +`RSBK` is opaque without our tool — so the inspectability loss is an accepted cost, +paid deliberately, not an oversight to be corrected later by reaching for ZIP. --- -## The container — three candidates (fork Ε-F1) +## The container — a proprietary `RSBK` (Ε-F1, RULED) -### (a) ZIP, via the vendored minizip +**Ruled by Daniel, 2026-08-02: *"proprietary container."*** The package is a +hand-rolled `RSBK` file. ZIP — whether via the vendored MiniZip64 in +`vendor/WDL/WDL/zlib/` or as a hand-written stored-only ZIP *shape* — is rejected and +is not to be revisited inside this phase. -`vendor/WDL/WDL/zlib/` vendors zlib **including MiniZip64** (`zip.c`, `unzip.c`, -`ioapi.c`). So a real ZIP writer/reader is available with no new third-party -dependency — only new sources compiled into the extension target. +**The shape.** Magic `RSBK`, a fixed little-endian header carrying the two version +fields (§"Version tagging" below), a length-prefixed JSON manifest, then each entry's +payload concatenated in manifest order. Framing overhead is tens of bytes, not +kilobytes. -- **For:** a user can open the package with any unzip tool and see the manifest and - the `.wav`s. Support value is genuine ("send me the package and I'll look - inside"). A standard format has a standard mental model. -- **Against:** minizip's API is path-and-file-handle shaped (`ioapi.h`), so the - codec cannot be pure — it drags the filesystem into the layer this project's - central discipline keeps free of hosts. Buffer-backed I/O is possible via a custom - `zlib_filefunc_def` but is fiddly and defeats the "standard" argument in the code - even while preserving it on disk. Compression buys almost nothing: the payload is - float32 PCM, which deflates poorly, and the manifest is kilobytes. A user can - hand-edit the archive and produce a package whose manifest and payload disagree — - a failure mode we then have to detect and explain. ~15 C files join the build. +**What it reuses, rather than invents.** The little-endian byte codec +(`core/wire/bytes.h` — `putLE` / `ByteReader`, called out in `src/core/wire/CLAUDE.md` +as the earned template case) and the hand-rolled JSON layer (`core/json`). Both are +already owned and already tested here. -### (b) A hand-rolled `RSBK` container — **recommended** +**Why the ruling went this way.** The load-bearing reason is not effort — it is that +the pure/shell split is this project's central discipline, and `RSBK` is the only +candidate where the **whole codec lands pure** and the shell is a bytes-in/bytes-out +skin. MiniZip's API is path-and-file-handle shaped (`ioapi.h`), so a ZIP codec drags +the filesystem into the layer that discipline keeps free of hosts; a buffer-backed +`zlib_filefunc_def` is possible but fiddly and defeats the "standard format" argument +inside the code even while preserving it on disk. Compression buys almost nothing on +float32 PCM. Writing a correct ZIP central directory by hand was the worst of both — +more code than `RSBK`, the same hardening burden, plus Zip64 and name-encoding edge +cases, and still no compression. -Magic `RSBK`, a fixed little-endian header carrying the two version fields, a -length-prefixed JSON manifest, then each entry's payload concatenated in manifest -order. +**The two costs, accepted with the ruling.** (1) The package is **opaque without our +tool** — no unzip-and-look support path. (2) We own the hostile-input hardening of our +own parser, to the discipline `bank_model::deserialize` and `parseLedger` already +carry — *error signaled, never UB* (`bank_model.h:204-206`). Both are priced in; a +later "let's make it inspectable" impulse is a new phase's argument, not this one's. -- **For:** the entire codec is pure `core/` — encode and decode are arithmetic over - bytes, unit-testable with no filesystem and no DAW. It reuses two things the - project already owns and tests: the little-endian byte codec - (`core/wire/bytes.h` — `putLE` / `ByteReader`, called out in - `src/core/wire/CLAUDE.md` as the earned template case) and the hand-rolled JSON - layer (`core/json`). Total control over the version ladder, which is the feature's - actual hard problem. Framing overhead is tens of bytes, not kilobytes. -- **Against:** opaque without our tool. We own the hostile-input hardening of our - own parser (mitigated: the same discipline `bank_model::deserialize` and - `parseLedger` already carry — "error signaled, never UB", `bank_model.h:204-206`). - No compression at all. - -### (c) ZIP *shape*, stored-only, hand-written - -Users can unzip it; we link nothing. Rejected: writing a correct ZIP central -directory by hand is more code than (b), inherits (b)'s hardening burden *and* ZIP's -edge cases (Zip64 past 4 GB, encoding of names), and returns no compression. It is -the worst of both. - -**Recommendation: (b).** The load-bearing reason is not effort — it is that the -pure/shell split is this project's central discipline, and (b) is the only option -where the whole codec lands pure and the shell is a bytes-in/bytes-out skin. The -inspectability that (a) buys is worth something; it is not worth a filesystem-coupled -codec plus a C library plus 15% on the one thing nobody wants compressed. -**This is a one-way door** — packages exist in users' hands the day it ships — so it -is Daniel's to ratify, not product-designer's to assume. +**This was a one-way door and it is now shut** — packages are in users' hands the day +it ships, and a later container change means either a second reader forever or +stranded packages. The version ladder below, not a format swap, is how the format +moves from here. --- @@ -321,6 +311,16 @@ guarantee for this feature; overselling it would be the error. ## Identity and collision on import +**The import target is settled first, because it frames all four collisions. +Ε-F2, RULED by Daniel, 2026-08-02: *"always lands as a new bank, with an auto suffix +if name collision."*** Every import creates a **new bank** in the destination book. +It never merges into an existing bank, never lands into the pool, never offers a +target picker, and never overwrites. **Merge-into-existing is out of scope for Phase +Ε** — not deferred behind a flag, not a second action shipped later in this phase, not +a checkbox. A user who wants imported samples in an existing bank imports and then +uses the existing move/copy verbs, which already do exactly that and already carry +their own undo. + Four distinct collisions hide under the word "collision," and they need four different answers. @@ -347,9 +347,83 @@ different answers. This is the one place the import must consult the model before touching the filesystem, and it is a concrete acceptance criterion rather than an optimization. 4. **Bank display name.** `bank_book` enforces unique display names, trimmed and - case-insensitive (`src/core/model/CLAUDE.md:21-25`), so `createBank("Drums")` into - a project that already has "Drums" is refused by the model. **Answer is fork - Ε-F2** — see below. + case-insensitive ASCII (`src/core/model/CLAUDE.md:22-26`; `createBank`'s own + contract at `bank_book.h:92-96` — *"Drums"/"drums"/" Drums " collide, including + against the pool's "Pool"*), so `createBank("Drums")` into a project that already + has "Drums" returns `false` with no mutation. **Answer: an automatic numeric + suffix, specified below.** No prompt, no overwrite, no refusal. + +### The auto-suffix rule (Ε-F2, implementation-binding) + +The importer picks the destination bank's display name itself. The user is told what +it picked; the user is never asked. + +**The seed.** The seed is the package's recorded source bank display name, taken +**verbatim**. If that name is absent, empty, or whitespace-only after the model's own +trim, the seed is the literal `Imported bank`. + +**The probe.** Let `seed` be that string and `fold(x)` be `BankBook`'s own uniqueness +key — strip leading/trailing ASCII whitespace, lower-case ASCII letters +(`bank_book.h:252-258`). Take the **first** name in this sequence whose fold is not +already carried by a bank in the destination book: + + seed, seed + " 2", seed + " 3", seed + " 4", … + +ascending from 2, unbounded. So `"Drums"` into a project already holding `"drums"` +lands as `"Drums 2"`; a third copy lands as `"Drums 3"`. + +**Four properties that make this unambiguous, each stated because omitting it lets two +implementations diverge:** + +1. **The seed is never re-parsed.** A package named `"Drums 2"` colliding in the + destination lands as `"Drums 2 2"`, **not** `"Drums 3"`. This is deliberate and is + not a defect to fix: a trailing integer cannot be distinguished from a user's own + name (`"Kit 808"` would become `"Kit 2"` under a stripping rule, silently losing + user-authored text). `resample_name::nextIterationName` may increment its tail only + because ` r` carries a marker; a bare integer carries none. Appending is the + safe direction — it never mutates text the user wrote. +2. **The probe fills gaps.** With `"Drums"` and `"Drums 3"` present and `"Drums 2"` + free, the import lands as `"Drums 2"`. First-free-ascending, not + highest-plus-one — the rule is a pure function of the destination's current name + set, so the same package into the same project always produces the same name. +3. **The suffix is derived from the destination, never from the package.** The package + records only its source name. Nothing about a collision is stored in the package, + and re-importing the same package into a different project can produce a different + name. The probe terminates: with `B` banks in the destination, one of the first + `B + 1` candidates is free by pigeonhole, so no cap is needed and none should be + added. +4. **The fold has exactly one home.** `import_plan` must **not** re-implement + `nameKey` — `bank_book.h:252-258` says in as many words that a drifted second copy + would let the uniqueness invariant be violated. The probe therefore runs behind + `BankBook`'s own folding, which means Ε-W2-T2 adds **one additive public `const` + member** to `BankBook` (recommended: `std::string uniqueDisplayName(const + std::string& seed) const`, returning the first free candidate) and calls it. That + one member is the only edit any Ε track makes to `core/model/`. + +**What the suffix does NOT touch.** It renames nothing but the new bank's display +name. Sample ids are reminted by collision rule 1 regardless of whether a name +collision occurred, and the two mechanisms are independent. **`Sample` display names +are never suffixed** — two banks may legitimately hold a sample called `"Kick"`, and +`resample_name`'s own contract already states that sample display names are not unique +(`resample_name.h:13-16`). Bank-folder file names are handled by collision rule 2 and +are unaffected by the bank's name. `slot_map` positions ride along unchanged. + +**The pool case is guaranteed, not hypothetical.** Exporting the pool is in scope (the +pool is structurally a bank), and the destination's pool always exists and always +carries the protected name `"Pool"`. So a pool export imported anywhere lands as a +**named** bank called `"Pool 2"`. That is correct under the Ε-F2 ruling — import never +lands into the pool — and it should read as intended behaviour in the summary, not as +a glitch. + +**What the user sees, and their recovery.** The import summary names the bank it +created, and says so plainly when the name was adjusted: + +> Imported 42 samples into a new bank: **Drums 2** (a bank named "Drums" already +> exists in this project). + +The recovery path is the existing rename verb — one Ctrl-Z undoes the whole import +including the bank creation, and a rename is one gesture if the user wants a different +name. Neither needs a new affordance. --- @@ -369,10 +443,10 @@ settled on: report before acting, and never leave a half-state that looks whole. | Entry name contains a path separator, `..`, or is absolute | import | Refuse whole, before any write | "This package is not well-formed." Hostile input, not user error — no need to elaborate | | Payload hash mismatch on any entry | import | Refuse whole, before landing anything | "This bank package is damaged (entry `` failed its integrity check). Nothing was imported." | | A write fails mid-import (disk full, permission) | import | Roll back: delete the files **this import wrote** and abandon the index mutation | "Import failed and was rolled back. Nothing was added." | -| Bank name collides in the destination | import | Fork Ε-F2 | See fork | +| Bank name collides in the destination | import | Auto-suffix, no prompt, no overwrite — first free of `seed`, `seed 2`, `seed 3`, … | Summary names the bank it created and says the name was adjusted | | File name collides in the bank folder | import | Auto-rename, no prompt | Counted in the summary line only | | Sample already present by content hash | import | Skip the write, collapse onto the existing entry | Counted in the summary line ("N already present") | -| Tracking ledger degraded at import time | import | Fork Ε-F3 | See fork | +| Tracking ledger degraded at import time | import | **Refuse whole**, before the picker's bytes are read and before any write — the guard runs first | The two-case message below, mirroring prune's abort | **On the rollback, and why it is not an invariant breach.** Prune is the single exclusive file-deletion authority, with exactly one carve-out, stated in one place — @@ -393,6 +467,88 @@ That is the same designed orphaned-until-prune window a non-empty bank delete al produces (`src/core/model/CLAUDE.md:38-40`). Say it out loud in the spec; do not let a user infer that Ctrl-Z cleans the folder. +### Import under a degraded tracking ledger (Ε-F3, RULED: refuse) + +**Ruled by Daniel, 2026-08-02: *"refuse mismatched import."*** An import that cannot be +cleanly reconciled against the tracking ledger is **refused outright**. There is no +confirm-and-proceed path, no "I understand the risk" checkbox, and no preference to +turn the guard off. This ruling went against the framing recommendation, and the +reasoning that carried it is recorded below rather than re-argued. + +**The trigger, exactly.** The guard fires when `tracking::ledgerDegraded(status)` holds +for the project's loaded ledger status — that is, `LedgerStatus::Unreadable` or +`LedgerStatus::FutureVersion` (`src/core/tracking/origin_ledger.h:94`, `:100-101`). +`Fresh` (absent key — a legitimate new project) and `Loaded` both proceed normally. + +**Two things the guard is deliberately NOT keyed on:** + +- **Not `PruneReport::blockedByTracking`.** That flag also fires on unreadable + `rsusage_*` keys, which are about live-instance protection during a *deletion*. + Import deletes nothing and computes no protected set; it *writes* birth records. An + undecodable usage key must not block an import, and reusing prune's composite flag + would silently make it do so. +- **Not the package.** Nothing in the `.rsbank` participates in this check. The package + is untouched by a refusal and remains importable later, elsewhere, or after the + project is repaired. + +**When it runs.** First — **before the file picker opens**, before a byte of the +package is read, before any allocation. Making the user find and pick a file we have +already decided to refuse is the wrong order. + +**What the user sees.** A console block through `ShowConsoleMsg`, mirroring prune's +abort (`src/shell/actions/prune_action.cpp:30-69`) in structure and in tone, because a +user who has hit prune's block should recognise this one. Every recovery line names +**this build's** ext-state namespace via `version::extStateNamespace()` — the +beta/stable trap prune already documents, where a beta user handed the stable spelling +clears the wrong key and is still blocked. Two cases, exactly one of which fires: + +*Malformed ledger:* + +> ReaSampler import: ABORTED — the file-tracking ledger could not be read. Nothing was +> imported. +> The stored file-tracking ledger is malformed. It has been left intact rather than +> overwritten, so it can be repaired or cleared: +> `reaper.SetProjExtState(0, "reasampler", "owned_files", "")` +> Clearing it makes every existing bank file un-reclaimable (they stop being +> attributable to ReaSampler); no file is lost. Reopen the project afterwards — the +> block is held for the rest of this session. +> An import can land hundreds of files in one gesture. With no readable ledger, none of +> them could be given a birth record, and every one would be permanently unreclaimable. + +*Ledger from a newer build:* + +> ReaSampler import: ABORTED — the file-tracking ledger could not be read. Nothing was +> imported. +> The stored file-tracking ledger was written by a NEWER version of ReaSampler than +> this one, so its records cannot be read safely. It has been left intact and will NOT +> be overwritten. Reopen the project with that newer version — do NOT clear this key +> from here, that would discard tracking records this build cannot see. The block is +> held for the rest of this session. +> An import can land hundreds of files in one gesture. With no readable ledger, none of +> them could be given a birth record, and every one would be permanently unreclaimable. + +**The recovery path.** The status is written only by `loadFromProject`, so it is sticky +for the session (`src/shell/persist/CLAUDE.md:39-46`): repair or clear the key +(malformed case only), or install the newer build (future-version case), **reopen the +project**, then import again. The package needs no re-export, and nothing about the +destination project was changed by the refusal. + +**Export is NOT gated on the ledger, and that asymmetry is intentional.** Export writes +no birth records, mutates nothing, and touches no ext state. A user whose ledger is +degraded can still get their bank *out* — which is exactly the moment they are most +likely to want to. Only the landing side refuses. + +**Why the ruling went this way.** The rejected option — allow the import behind an +up-front confirm — matched the accepted residual already stated at +`core/tracking/CLAUDE.md:24-31`, where a capture made during a degraded session is +recorded in memory but not persisted and degrades to foreign. The argument that carried +is **scale**: that residual contemplates *one* untracked capture, and a bulk import can +strand two hundred files in a single gesture. Same mechanism, different animal. A +confirm would also push a data-lifecycle consequence onto the user at the one moment +they are least equipped to evaluate it — mid-transfer, wanting the samples. The refusal +costs a retry after a project reload; the confirm costs a bank folder that can never be +reclaimed. + --- ## Memory: the streaming seam that keeps the codec pure @@ -526,39 +682,26 @@ constructors. --- -## Open forks — Daniel's +## Rulings — Daniel's, 2026-08-02 -**Ε-F1 — container format. Recommendation: (b) hand-rolled `RSBK`.** -Alternative: (a) ZIP via vendored minizip, buying user-inspectability at the cost of a -filesystem-coupled codec and ~15 C files in the build. **One-way door** — the format -is in users' hands the day it ships, and a later change means either a second reader -forever or stranded packages. Ratification wanted before Ε-W1-T1 is dispatched. +All three [Daniel]-class forks this doc opened were ruled the same day it was framed. +Nothing here is open. This section is an index; each ruling is **specified** in the +section that owns it, and that section is the implementation-binding text. -**Ε-F2 — import target: new bank always, or offer merge-into-existing?** -Recommendation: **a new bank by default**, named from the package's recorded bank -name, with a prompt pre-filled with a uniqueness-folded suggestion when that name is -taken ("Drums" → "Drums 2"). Merging into an existing bank is a genuinely different -intent and should be a genuinely different gesture — a separate "import into the -active bank" variant, not a checkbox on one dialog. Rationale: the default stays -non-destructive and legible, and the two verbs stay distinguishable in the Actions -list, which matters because they are both bindable. Counter-argument worth hearing: -two actions for one feature is more surface, and a user importing a bank they already -have a copy of will find the default annoying. Daniel's call on whether one verb or -two ships. +| Fork | Ruling | Specified in | +|---|---|---| +| **Ε-F1** | **Proprietary container.** Hand-rolled `RSBK`. ZIP via the vendored MiniZip64, and a hand-written stored-only ZIP shape, are both rejected | §"The container" | +| **Ε-F2** | **Import always lands as a new bank**, with an automatic numeric suffix on a display-name collision. Merge-into-existing is out of scope for this phase | §"Identity and collision on import" — the frame, plus the auto-suffix rule | +| **Ε-F3** | **Refuse** an import while the tracking ledger is degraded. No confirm-and-proceed path | §"Import under a degraded tracking ledger" | -**Ε-F3 — import while the tracking ledger is degraded.** When the ledger is -`Unreadable` or `FutureVersion`, `ext_state_io` deliberately **skips** the -`owned_files` write (`src/shell/persist/CLAUDE.md:39-46`) so a blob it could not read -is never replaced by a truncation. Consequence: files landed during such a session get -no birth record and become permanently unreclaimable foreign files. -Recommendation: **allow the import, behind an explicit up-front confirm that names -the consequence** — this matches the accepted residual already stated at -`core/tracking/CLAUDE.md:24-31` rather than inventing a new block. Alternative: -**refuse**, matching prune's block. The argument for refusing is scale: prune's -accepted residual contemplates *one* untracked capture, and a bulk import can strand -two hundred files in one gesture, which is a different animal even if it is the same -mechanism. This is a data-lifecycle policy call, not an implementation detail, and it -should be ruled rather than defaulted. +**Two of the three went to a different answer than the framing recommended, and the +reasons are worth keeping.** Ε-F2's recommendation was a *prompt* pre-filled with a +uniqueness-folded suggestion; the ruling removed the prompt entirely, which is the +better shape — the name is derived deterministically from the destination, the user is +told rather than asked, and the existing rename verb is the recovery. Ε-F3's +recommendation was allow-with-confirm; the ruling refused, and the counter-argument +raised alongside that recommendation is what carried it (scale — the accepted residual +contemplates one untracked capture, an import strands hundreds). --- @@ -569,8 +712,11 @@ should be ruled rather than defaulted. - **No partial import.** All-or-nothing, with rollback. A partially-imported bank is the failure mode this whole design is shaped to avoid. - **No re-encode, no trim, no normalize, no mono collapse on either side.** -- **No compression of the audio payload** (regardless of how Ε-F1 lands — if ZIP wins, - entries are stored, not deflated; the manifest may compress). +- **No compression of the audio payload.** `RSBK` concatenates payload bytes; there is + no compressor in the path and none is to be added. +- **No merge-into-existing import.** Every import creates a new bank (Ε-F2). There is + no target picker, no "import into the active bank" variant, and no second action. + Move/copy already move samples between banks after the fact. - **No instrument state in the package** — the seam is left additive, deliberately unspent. - **No whole-book export in this phase.** One package carries one bank, because that