From 86c3c7f3b865614201f7d2d8daa33eae93617e2b Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 06:32:07 -0400 Subject: [PATCH] =?UTF-8?q?docs:=20spec=20Phase=20=CE=95=20=E2=80=94=20ban?= =?UTF-8?q?k=20export/import=20as=20a=20version-tagged=20package?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds docs/product/bank-package.md and the Phase Ε spec in docs/PLAN.md: three waves, six tracks. Three forks open; Ε-F1 blocks Ε-W1-T1. --- docs/PLAN.md | 542 +++++++++++++++++++++++++++++++- docs/product/bank-package.md | 582 +++++++++++++++++++++++++++++++++++ 2 files changed, 1122 insertions(+), 2 deletions(-) create mode 100644 docs/product/bank-package.md diff --git a/docs/PLAN.md b/docs/PLAN.md index b9c02a7..4f26733 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -7,7 +7,9 @@ dispatched against directly — **plus Phase Γ**, which did not come from those `docs/product/instrument-control-surface.md`, **and Phase Ψ**, which likewise did not come from the seventeen: it came from a direct list of seven defects and refinements (Daniel, 2026-08-01) and is specified inline in its own section below — there is no -backing product doc for it. +backing product doc for it, **and Phase Ε**, which likewise did not come from the +seventeen: it came from a direct request (Daniel, 2026-08-02) and is scoped in +`docs/product/bank-package.md`. ## What this doc is, and how it relates to the others @@ -28,7 +30,7 @@ backing product doc for it. cited section rather than reading a file whole. **Worktree slug convention:** `p-w-t-`. Greek phase letters -transliterate: **Θ → `th`**, **Ξ → `xi`**, **Γ → `g`**, **Ψ → `psi`**. So Θ-W1-T1 +transliterate: **Θ → `th`**, **Ξ → `xi`**, **Γ → `g`**, **Ψ → `psi`**, **Ε → `e`**. So Θ-W1-T1 dispatches into `pth-w1-t1-zone-retirement`, Γ-W1-T1 into `pg-w1-t1-knob-interaction-law`, and Ψ-W1-T1 into `ppsi-w1-t1-capture-range-exactness`. @@ -60,6 +62,14 @@ the stage lengths to 10s"* — the ceiling moves in Γ-W1-T1. **Γ-F7** (the VST *order*) is **RULED: signal flow** — *"signal flow order."* **There is now NO unanswered [Daniel]-class question anywhere in this plan.** +**That claim is scoped to Phases Θ / Ξ / Γ / Ψ, and Phase Ε reopens the class.** Phase Ε +(added 2026-08-02) carries **three** [Daniel]-class forks — **Ε-F1** (container format), +**Ε-F2** (import target), **Ε-F3** (import under a degraded ledger) — each stated with a +recommendation and its counter-argument in the phase header below and in +`docs/product/bank-package.md` §"Open forks". **Only Ε-F1 blocks a dispatch** (Ε-W1-T1); +the other two are answerable at Ε-W2 and could be ruled at implementation review if Daniel +prefers, but both are user-visible policy rather than implementation detail. + **Ruling 3 (Daniel, 2026-08-01) — real units at the host boundary.** *"The parameter values exposed to the VST host should be in real units, such that the host automation lanes report usable values."* Satisfied through VST3's **plain-value layer**, not its wire format (which is @@ -2243,6 +2253,504 @@ verification script on this branch, for this track's multi-track refusal specifi --- +## Phase Ε — The bank package: one file that carries a bank between projects + +**Ships:** a bank exported to a single version-tagged `.rsbank` file — audio bytes +byte-exact, index metadata intact — and imported into another project's bank folder and +index, with a compatibility policy that names both directions concretely: an older package +in a newer build always imports, a newer package in an older build refuses whole with an +actionable message, and neither direction is ever a partial landing. + +**Consolidates: none of the seventeen.** Phase Ε came from a direct request (Daniel, +2026-08-02) and is scoped in `docs/product/bank-package.md`. Nothing in `docs/TODO.md` or +`docs/TODO-1.0.md` records export, import, or a package format — a sweep of `docs/` for +`export|package|portable` returns only unrelated matches (prune's portable move-to-trash, +MIDI-playback prose) — so this phase **supersedes nothing and absorbs nothing.** + +**Three forks are OPEN and are Daniel's.** They are the first unanswered [Daniel]-class +questions in this plan since Γ closed its seven; each is stated with a recommendation and +its counter-argument in `docs/product/bank-package.md` §"Open forks". + +| Fork | Question | Recommendation | Blocks | +|---|---|---|---| +| **Ε-F1** | Container format: hand-rolled `RSBK` vs. ZIP via the vendored minizip (`vendor/WDL/WDL/zlib/`) | **hand-rolled `RSBK`** — the only option where the whole codec lands pure | **Ε-W1-T1 dispatch.** One-way door: packages are in users' hands the day it ships | +| **Ε-F2** | Import target: always a new bank, or also offer merge-into-existing | **new bank by default**, merge as a *separate* bindable verb if wanted at all | Ε-W2-T2's action count and dialog; not W1 | +| **Ε-F3** | Import while the tracking ledger is degraded: allow-with-confirm, or refuse | **allow with an up-front confirm naming the consequence** — matches the accepted residual rather than inventing a new block | Ε-W2-T2's guard; not W1 | + +**Ε-F1 is the only one that blocks a dispatch.** Ε-F2 and Ε-F3 can be carried into Ε-W2-T2 +as ruled-at-review if Daniel prefers, but both are user-visible policy rather than +implementation detail, so the plan's default is to ask. + +### Phase-Ε acceptance criteria + +These bind every track in this phase, in addition to the plan-wide set above. + +- **Byte-exact round-trip is the phase's trust anchor.** Export → import → export yields + byte-identical payloads, and the `hashBytes` digest of every landed file equals the digest + recorded at export. Frame count, sample rate, bit depth, and channel count are untouched on + both sides. **No re-encode anywhere:** `wav_codec` may be called to hash and to read + metadata already recorded, never to rebuild, trim, normalize, or collapse. The mono collapse + is a capture-path behaviour and must not reach the import path — the same exclusion ingest + already carries (root `CLAUDE.md`, exact-bounds invariant). +- **The format cannot express a path.** Manifest entries are bare file names — no directory + component, no `..`, no drive letter, no leading separator — validated on encode *and* + decode. Relative-paths-only becomes structural rather than remembered, and the + archive-traversal bug class closes by construction. +- **Import places no timeline item.** Capture and placement stay separate acts; import is a + capture-shaped act, not a placement one. +- **Import never overwrites and never deletes an existing bank-folder file.** The one + deletion path is the rollback of files *this call wrote* that no index ever referenced — + the documented carve-out at `src/shell/persist/prune_fs.cpp:5-11`, which every track + touching it must **cite, not restate**. +- **Export is read-only against the project.** No ext-state write, no `bumpBankGeneration()`, + no undo point. Import does the opposite: it bumps the generation + (`src/shell/persist/session.h:108`) so live ReaSampler 9000 instances reload, and batches + its index mutation into one Ctrl-Z through `persistBankOp`. +- **All-or-nothing on both sides.** No partial export, no partial import. A truncated + `.rsbank` must never exist on disk (temp file + atomic rename, the Ψ-W2-T2 precedent); a + half-imported bank must never exist in the index (rollback). +- **At most one entry's payload in memory at a time**, on both paths. The pure codec owns + framing and offset arithmetic; the shell owns the stream. A whole-package + `vector` on either side is a rejected shape, not an optimization opportunity. +- **The pure planners take value inputs, never a handle.** The decoded manifest, the + destination `BankBook`, and the set of names present in the bank folder cross the seam as + values. No `ReaSamplerSession&`, no service container, no "pass the thing that has + everything" reaches `core/package/`. If a circular dependency appears during the build, the + fix is a service split or a thin interface — **not** parameter propagation, and **not** a + base class gaining a dependency that grows its subclasses' constructors. +- **Every pure module gets a `_tests` target** that runs without REAPER or a DAW. + The whole collision/version rule set is expressible as pure functions over strings and + hashes; if a rule can only be tested through the shell, the seam is in the wrong place. + +**Performance posture.** Every surface in this phase is cold — per-gesture, once. None of +the named hot paths (peaks envelope compute, audition, realtime-capture tick, instrument +`process()`) is touched by any track here. The one performance fact that *is* load-bearing is +the memory criterion above, and it is stated as a structural constraint rather than a +guardrail because exceeding it does not slow the feature down, it makes it fail. + +**Concurrency with Γ and Λ.** Phase Ε is extension-side and lands almost entirely in **two +new directories** (`src/core/package/`, `src/shell/package/`) that no other phase touches. +Γ lives in `core/instrument/` + `shell/instrument/`; Λ is being specced concurrently and is +not read here. The only pre-existing files any Ε track edits are named per track below — +`core/tracking/origin_ledger` (W1-T3, exclusively), the root `CMakeLists.txt` +`add_subdirectory` list (W1-T1 and W1-T2, one line each), `src/app/main.cpp` and the panel's +bank menu (W2-T1 and W2-T2, one registration line and one menu row each). **No Ε track +touches `core/instrument/`, `shell/instrument/`, or any capture backend.** + +--- + +### Ε-W1 — The contract, the filesystem, and the ledger's new kind + +**Depends on:** nothing in this phase. **Three tracks, disjoint by directory** — the split is +by *what each track's inputs are*, which is why they genuinely parallelize: T1 knows only +bytes and structs, T2 knows only paths and bytes, T3 knows only the ledger. + +| Track | Owns | +|---|---| +| **T1** `package-format` | the whole of the new `src/core/package/` **except** `export_plan` / `import_plan` (W2's), plus its `CMakeLists.txt` and `CLAUDE.md` | +| **T2** `package-fs-shell` | the whole of the new `src/shell/package/` **except** `export_bank` / `import_bank` (W2's), plus its `CMakeLists.txt` and `CLAUDE.md` | +| **T3** `import-origin-kind` | `src/core/tracking/origin_ledger.{h,cpp}` and its tests, exclusively | + +**One shared file in the wave, named rather than discovered at merge:** the root +`CMakeLists.txt` `add_subdirectory` list — T1 appends `src/core/package`, T2 appends +`src/shell/package`. Two append-only lines in one list: **textual merge adjacency, not +semantic contention.** Whichever lands second rebases. + +**T1 is the wave's only gated dispatch** — it cannot start before Ε-F1 is ruled, because the +fork *is* T1's deliverable. T2 and T3 are unaffected by Ε-F1 and can start immediately: T2's +API is bytes-in/bytes-out regardless of what those bytes mean, and T3 touches no package +code at all. + +#### Ε-W1-T1 — `package-format` + +**Goal.** The container and its version ladder, entirely pure — the contract every later +track consumes, landed once so nothing downstream re-litigates the shape. + +**Spec:** `docs/product/bank-package.md` §"The container", §"Version tagging", §"What a +package carries", §"What a package deliberately does NOT carry", §"Memory". + +**Surface boundary — owns:** new `src/core/package/package_format` (the magic, the header +layout, `kPackageFormatVersion`, `kPackageMinReaderVersion`, and +`classifyPackageVersion(formatVersion, minReader) -> Readable | TooNew | Malformed`), new +`src/core/package/package_manifest` (the manifest model + its JSON codec), new +`src/core/package/bank_package` (header encode, prefix decode, entry-layout arithmetic), the +directory's `CMakeLists.txt` and `CLAUDE.md`, and one appended `add_subdirectory` line in the +root `CMakeLists.txt`. **Does not own:** `export_plan` / `import_plan` (Ε-W2), anything under +`shell/`, `core/model`, or `core/tracking`. + +**Behavior.** +- **Two version integers, not one.** `formatVersion` = what this writer emitted; + `minReaderVersion` = the oldest reader that can read it safely. The reader's whole rule is + `minReaderVersion <= kPackageFormatVersion`. An **additive** change (a new optional + manifest key, a new enum value with a defined degrade) bumps `formatVersion` only; a + **structural** change bumps both. The header carries the writer's semver + (`version::stampVersion()`) alongside them, informational, so a refusal message can name + what to install. +- **The ladder is documented the way `origin_ledger.cpp:8-21` documents its own** — a header + comment listing every shipped version and what changed, with the read-and-validate rule + stated, not implied. +- **Unknown manifest keys are skipped** (the `bank_book_json.cpp:182` behaviour), and + **unknown persisted enum integers degrade to their defined `Unknown` equivalent**, never to + the numeric default and never to a parse failure (`core/wire/CLAUDE.md`'s `BakeStatus` rule, + verbatim). Both are pinned by tests, not left to inheritance. +- **The manifest nests `BankModel`'s own serialization verbatim**, exactly as + `bank_book_json.cpp:15-20` nests it, so per-sample shape has one owner and a future + `Sample` field reaches packages for free. Per entry the manifest adds only: the bare file + name, the byte length, and a `hashBytes` digest (`core/capture/wav_codec.h:143`) — + `hashBytes`, **not** `hashWavContent`, because the latter deliberately skips chunks + (`wav_codec.h:145-151`) and so cannot answer "did these bytes survive." +- **The bank's `slot_map` rides along** — display positions are part of what the user built. +- **Framing only, never a payload.** `bank_package` produces the header bytes and an ordered + `[{ name, offset, length }]` layout; it never holds, copies, or hashes an entry's audio. + Decode is symmetric: prefix in, manifest + layout out. +- **Path expression is structurally impossible.** Entry names are validated to contain no + `/`, `\`, `:`, no leading separator, and no `..` component, on both encode and decode. + +**Acceptance criteria.** +- `decodePackage(encodePackage(x)) == x` over a manifest fixture exercising every field, + including every `Sample` optional in both present and absent states. +- A synthetic header with `minReaderVersion` above this build classifies `TooNew` and **no + manifest is produced** — the decode does not half-succeed. +- A synthetic header with `formatVersion` above this build but `minReaderVersion` at or below + it classifies `Readable`, and its unknown manifest keys are skipped without error. This is + the additive-forward-compatibility claim, and it is the reason the two-integer design + exists; a test that does not exercise it leaves the design unproven. +- Truncated input at every byte offset in a valid package returns `Malformed` — never UB, + never a partial manifest, never a read past the buffer. Hostile-input hardening at the + `bank_model.h:204-206` standard. +- Entry names containing `..`, a separator, or an absolute prefix are rejected on encode + *and* rejected on decode. Both directions, because a package can arrive from anywhere. +- No file in the new directory exceeds ~600 lines; the three-module split above is the + responsibility seam, and a fourth module is preferred over a bisection if one is needed. +- `package_format_tests`, `package_manifest_tests`, `bank_package_tests` all run without + REAPER or a DAW. + +**Open questions.** **[Daniel] Ε-F1** — the container format itself; this track cannot be +dispatched until it is ruled. **[propose at review]** whether `package_format` and +`bank_package` are genuinely two modules or one — the split is proposed on responsibility +grounds (constants and classification vs. offset arithmetic) and may collapse if the +arithmetic turns out to be twenty lines. + +#### Ε-W1-T2 — `package-fs-shell` + +**Goal.** Every filesystem and dialog act the two verbs need, landed behind an API that knows +nothing about what a package contains — so it can be authored, reviewed, and tested in +parallel with the format it will carry. + +**Spec:** `docs/product/bank-package.md` §"Where it lives", §"Failure modes", §"Memory". + +**Surface boundary — owns:** new `src/shell/package/package_io` (read a file's bytes, write +bytes through temp + atomic rename, read one bank file, write one landed file, enumerate the +bank folder's existing names, and the rollback delete), the platform file-picker seam under +the `#ifdef _WIN32` / `#else swell/swell.h` split this codebase already uses +(`src/shell/panel/draw_kit.cpp:11-15`, `src/shell/persist/prune_fs.cpp:35-38`), the +directory's `CMakeLists.txt` and `CLAUDE.md`, and one appended `add_subdirectory` line in the +root `CMakeLists.txt`. **Does not own:** `export_bank` / `import_bank` (Ε-W2), anything under +`core/`, and — emphatically — `prune_fs`, which stays the deletion authority. + +**Behavior.** +- **Atomic write.** A package is written to a temp path in the destination directory and + renamed on complete success. A failed or interrupted write leaves no `.rsbank` behind. This + is the mono-collapse precedent (Ψ-W2-T2, temp file + atomic rename) applied to a much + larger file. +- **Streaming, both ways.** Append one payload at a time on write; seek and read one payload + at a time on read. The API must make holding the whole package awkward, not merely + discouraged. +- **The rollback delete is the carve-out, cited.** Its TU header cites + `src/shell/persist/prune_fs.cpp:5-11` and states the discriminator it satisfies — this call + created the file, and no index ever referenced it — rather than restating the carve-out's + text. Anything that does not satisfy that discriminator is not this function's business. +- **The two pickers are asymmetric, and the asymmetry is real.** Import uses REAPER's own + `GetUserFileNameForRead(char* filenameNeed4096, const char* title, const char* defext)` — + **verified**, `vendor/reaper-sdk/sdk/reaper_plugin_functions.h:3798`. There is **no save + picker in the REAPER API** — a sweep of that header for `FileNameFor|SaveFile|Browse` + returns only the read picker — so export uses Win32 `GetSaveFileNameW` on Windows and + SWELL's `BrowseForSaveFile` elsewhere (`vendor/WDL/WDL/swell/swell-functions.h:167`). +- **No REAPER project state is touched here.** No ext-state read or write, no undo block, no + generation bump; those belong to the verbs in Ε-W2. + +**Acceptance criteria.** +- A write interrupted before completion leaves the destination path absent or holding its + prior contents — never a partial new file. Tested by injecting a failure at the writer seam. +- Reading and writing a multi-entry package never holds more than one entry's payload; the + test asserts against a seam counter, not against a memory measurement. +- The rollback deletes exactly the files it was given and nothing else, and is a no-op on a + path it did not write. +- The Windows and SWELL picker paths both compile; the SWELL signature matches + `swell-functions.h:167` exactly. `[verify — DAW]` — neither picker is exercised in a live + REAPER session by this track. +- No file exceeds ~600 lines; the platform picker lives in its own TU, following the + `drag_out` / `drag_out_win` precedent. + +**Open questions.** **[propose at review]** whether the file-picker seam is its own module or +part of `package_io` — the `drag_out_win` precedent argues its own TU; whether it also wants +its own header is a judgment call at the size it lands. **[verify — DAW]** the default +extension and filter strings each platform's picker actually accepts. + +#### Ε-W1-T3 — `import-origin-kind` + +**Goal.** Give the ledger a birth-record kind for a package import, so an imported file is +tracked from the moment it lands rather than becoming a permanently unreclaimable foreign +file — landed as its own track, with its own review, because it edits safety-critical +territory that nothing else in this phase touches. + +**Spec:** `docs/product/bank-package.md` §"What a package deliberately does NOT carry" (the +origin-ledger bullet); `src/core/tracking/CLAUDE.md` for the invariants it must not weaken. + +**Surface boundary — owns:** `src/core/tracking/origin_ledger.{h,cpp}` and its tests, +exclusively. **Does not own:** `tracking_authority` (no decision changes), `shell/persist`, +or any consumer. + +**Behavior.** +- **Append `OriginKind::PackageImport` as value 5.** Append only — `Capture`=1, `Ingest`=2, + `Recapture`=3, `Resample`=4 keep their integers, per `core/tracking/CLAUDE.md`'s + "PERSISTED INTEGERS — never renumber, only append". +- **A build that does not know value 5 degrades it to `Unknown`**, which is the existing + `kindFromInt` behaviour and is the safe direction: the path is still owned, so still + protected; only the kind detail is lost. This is the *field-vocabulary* rule, and it must + stay distinct from the *document-version* rule right beside it, which blocks + (`origin_ledger.cpp:18-21`). +- **Nothing else changes.** No new field, no version bump, no lineage semantics. A new enum + value in an append-only vocabulary is precisely the change that does **not** need `"v"` to + move, and demonstrating that is part of the point. +- **`recordCreated` needs no signature change** — it already takes an `OriginKind` + (`src/shell/persist/session.h:95`). Confirm that in the same pass; if it turns out + otherwise, that discovery is this track's, not Ε-W2's. + +**Acceptance criteria.** +- A ledger containing a kind-5 record round-trips through serialize/deserialize unchanged. +- A record carrying an *unrecognized* kind integer (6, 99, negative) loads as `Unknown` and + the ledger loads `Loaded`, not `Unreadable` — the vocabulary gap does not halt prune. +- `kLedgerVersion` is **unchanged** at 2, and a test asserts it, so the append-vs-bump + distinction is pinned rather than assumed. +- `pruneProtection`'s output is unchanged for every existing kind — this track alters no + decision. + +**Open questions.** **[propose at review]** whether the kind is named `PackageImport` or +folded onto the existing `Ingest`. The plan's recommendation is a distinct value: `Ingest` +means "the user brought in a file," which is close, but losing the distinction makes a future +"where did this bank come from" question unanswerable, and an appended integer costs nothing. + +--- + +### Ε-W2 — The two verbs + +**Depends on Ε-W1 — all three tracks.** T1 for the format the verbs speak, T2 for every +filesystem act they perform, T3 for the kind their birth records carry. No part of either +verb is authorable against a format that has not settled. + +**Two tracks, disjoint by direction.** They share only the manifest type. The split is real +enough that the plan **pre-split the pure planner into two TUs** (`export_plan` / +`import_plan`) rather than one `package_plan` — that separation exists specifically so these +two tracks do not fight over a file. + +| Track | Owns | +|---|---| +| **T1** `bank-export` | `core/package/export_plan`, `shell/package/export_bank`, `shell/actions/package_export_action` | +| **T2** `bank-import` | `core/package/import_plan`, `shell/package/import_bank`, `shell/actions/package_import_action`, the panel's `.rsbank` drop route | + +**Two shared files, named — and the disjointness here is CONDITIONAL, unlike W1's.** +`src/app/main.cpp` (one action-family registration line each) and the panel's bank menu (one +row each). Both are textual adjacency by construction, but Phase Ψ set the precedent of +granting `main.cpp` to a single track rather than sharing it (Ψ-W1-T3). **If the dispatcher +wants zero contention, serialize T2 behind T1** — T2 is the larger track and loses nothing by +starting second. The plan's default is to run them in parallel and rebase whichever lands +second. + +#### Ε-W2-T1 — `bank-export` + +**Goal.** One bank leaves the project as one file, or the export refuses and says why. + +**Spec:** `docs/product/bank-package.md` §"Failure modes" (export rows), §"What a package +carries". + +**Surface boundary — owns:** new `core/package/export_plan` (pure: which entries, what +names, what is missing, and therefore whether the export may proceed), new +`shell/package/export_bank` (the promptless verb — takes a `ReaSamplerSession&`, returns an +outcome, **no prompts and no message boxes**, mirroring `src/shell/bank_ops/`), new +`shell/actions/package_export_action` (the bindable-action skin, mirroring `prune_action`), +one registration line in `src/app/main.cpp`, one panel menu row. **Does not own:** anything +on the import side, `bank_ops`, or `persist`. + +**Behavior.** +- **The exported unit is one bank** — the pool included, since the pool is structurally a bank + (`core/model/CLAUDE.md`'s pool-privileges section). Whole-book export is an explicit + non-goal of this phase and is preserved as an additive future by the manifest's shape, not + by a promise. +- **Refuse-if-incomplete, report-before-acting.** An index entry whose file is missing or + unreadable stops the export by default; the "export the N present entries" path exists only + behind an explicit confirm that lists what is absent, distinguishing missing from + unreadable. This is prune's dry-run-then-confirm discipline applied to a non-destructive + act, and it is deliberate: a silently-incomplete package is discovered on the far side, in + another project, weeks later. +- **The project is not touched.** No ext-state write, no generation bump, no undo point. An + export that mutates project state is a defect, and the acceptance criteria name it as one. +- **Nothing is re-encoded.** Payload bytes are copied and hashed. `wav_codec` is not asked to + rebuild anything. +- **A new FOREVER-STABLE command id** is minted through `version::channelCommandId(suffix)` + per the root `CLAUDE.md` action contract, with its per-channel display name through + `channelActionName`. + +**Acceptance criteria.** +- `planExport` is pure and total over its inputs: a bank with a missing file, an unreadable + file, zero samples, and one sample all classify without touching a filesystem. +- Exporting a bank and re-reading the package yields, for every entry, a `hashBytes` digest + equal to the source file's — asserted per entry, not in aggregate. +- The exported manifest contains no absolute path and no path separator, asserted by a test + that scans the emitted bytes rather than by inspecting the model. +- A failure injected mid-write leaves no `.rsbank` at the destination and the prior file, if + any, intact. +- Project ext state is byte-identical before and after an export, and `bankGeneration()` is + unchanged — a direct assertion, because "we did not mean to write anything" is not a + property that survives without one. +- Exporting an empty bank produces a valid, importable package with zero entries rather than + refusing. An empty bank is a legitimate thing to carry. + +**Open questions.** **[propose at review]** whether the export affordance is action-only, +panel-only, or both at ship. **[propose at review]** whether the default file name is derived +from the bank's display name (recommended, sanitized through +`capture_paths::sanitizeStem`) or from the project name. + +#### Ε-W2-T2 — `bank-import` + +**Goal.** A package becomes a bank in this project — completely, or not at all — with every +one of the four collision classes answered explicitly rather than by whatever the model +happens to do. + +**Spec:** `docs/product/bank-package.md` §"Identity and collision on import", §"Failure +modes" (import rows), §"Version tagging: both directions". + +**Surface boundary — owns:** new `core/package/import_plan` (pure: the id remap table, the +parent remap, the per-entry write / skip-already-present / rename disposition, the +destination bank name after uniqueness folding), new `shell/package/import_bank` (the +promptless verb), new `shell/actions/package_import_action`, the panel's `WM_DROPFILES` route +for a `.rsbank` (routing only — the existing ingest route for audio files is untouched), one +registration line in `src/app/main.cpp`, one panel menu row. **Does not own:** anything on the +export side, `bank_book`'s rules (consumed, never re-implemented), `origin_ledger` +(Ε-W1-T3's). + +**Behavior.** +- **Version gate first, before any byte is written.** `minReaderVersion` above this build + refuses the whole package and reports through `ShowMessageBox` + (**verified**, `reaper_plugin_functions.h:6546`) naming three things: the package's + requirement, this build's ceiling, and the writer's semver. Two of the three is not enough + to act on. A malformed or truncated package reports **distinctly** — the two failures have + opposite recoveries, which is exactly why `origin_ledger.cpp:178-185` separates them. +- **Four collisions, four answers.** (1) **Sample id** — remint every id and remap + `Provenance::parentSampleId` (`bank_model.h:45-50`) through the same map, to the reminted + parent when it came in the same package and cleared otherwise; a foreign id never enters + the index. (2) **File name** — never overwrite; mint a fresh unique name through + `capture_paths::deriveBankPaths`, silently, counted in the summary. (3) **Content hash** — + consult `BankModel::findByHash` **before writing the payload**; on a hit, skip the write + entirely and let the entry collapse, so a dedup never manufactures an orphan. (4) **Bank + display name** — Ε-F2. +- **Birth records at landing.** Every landed file goes through + `ReaSamplerSession::recordCreated(sample, OriginKind::PackageImport)` at the same point the + `Sample` is added, in the same straight-line block, per `core/tracking/CLAUDE.md`'s + no-silent-gaps invariant. An import that lands a file without a record is the exact failure + that section exists to prevent. +- **All-or-nothing, with rollback.** Any failure after the first write deletes the files + *this call wrote* and abandons the index mutation. The rollback cites the + `prune_fs.cpp:5-11` carve-out; it does not restate it, and it does not reach outside the set + it wrote. +- **One Ctrl-Z for the index, and the file residue is stated, not implied.** The index + mutation batches through `persistBankOp` (`Undo_BeginBlock2` / `Undo_EndBlock2`, verified at + `reaper_plugin_functions.h:7758` / `:7806`). Undo does **not** un-write the files; they + remain as orphans until a prune reclaims them — the same designed window a non-empty bank + delete already produces (`core/model/CLAUDE.md`'s sample-removal section). The user-facing + summary says so. +- **`bumpBankGeneration()` on success** (`session.h:108`), so live instances reload. +- **No timeline item is placed. Ever.** +- **A new FOREVER-STABLE command id**, minted the same way T1's is. + +**Acceptance criteria.** +- `planImport` is pure and total, and every one of the four collision classes has a test that + exercises it without a filesystem: colliding ids, colliding file names, a hash already + present, and a colliding bank name. +- Importing a package built from bank B into a project that already contains B produces a + correct result under the Ε-F2 ruling, with every id reminted and no entry lost. +- An entry whose payload fails its `hashBytes` check aborts the import with **zero** files + landed and **zero** index mutation — asserted on both, since either alone would pass a + weaker test. +- A write failure injected at entry k of n leaves exactly zero files from this import on + disk and the index unchanged. +- Every landed file has a ledger birth record with kind `PackageImport`, asserted by reading + the ledger after the import, not by counting calls. +- `minReaderVersion` above the build: nothing written, message names all three facts. + `formatVersion` above the build with `minReaderVersion` at or below it: **imports cleanly**, + unknown keys skipped. Both directions asserted, in this track, against real package bytes. +- No timeline item exists after an import; the arrange is byte-identical. +- **DAW-verification obligation** (to be discharged by Daniel, not by this track): import a + package produced on another machine, confirm the panel shows every sample with its + metadata, confirm a live ReaSampler 9000 instance picks up the new bank content on the + generation bump, and confirm one Ctrl-Z removes the index entries. + +**Open questions.** **[Daniel] Ε-F2** — new bank always, or merge offered; decides whether +this track ships one action or two. **[Daniel] Ε-F3** — behaviour when the ledger is degraded +at import time. **[propose at review]** whether the import summary is a console block, a +message box, or both; the recommendation is a console block plus a one-line message box, so +the detail is copyable and the outcome is unmissable. + +--- + +### Ε-W3 — The compatibility fixtures + +**Depends on Ε-W2 for:** both verbs existing. A round-trip claim cannot be tested against one +half of a round trip, and a "this build refuses a future package" claim cannot be tested +against a package this build is incapable of writing. + +**One track.** The whole deliverable is one corpus and the harness over it; splitting it +would mean two tracks writing two halves of one fixture set. + +#### Ε-W3-T1 — `package-compat-fixtures` + +**Goal.** Turn the version-compatibility policy from an assertion in a doc into a property +proven against **frozen bytes**, so a later format change cannot silently break either +direction. + +**Spec:** `docs/product/bank-package.md` §"Version tagging: both directions". + +**Surface boundary — owns:** a new checked-in fixture corpus under the package modules' test +tree, the harness that decodes it, and one `docs/` verification script for the DAW half. +**Does not own:** any production module — if a fixture reveals a defect, the fix is filed +against the owning track's module and this track carries the failing test, not the patch. + +**Behavior.** +- **Frozen bytes, not regenerated ones.** The corpus holds real `.rsbank` bytes committed to + the repo: a v1 package written by the shipping build, a synthetic + `formatVersion` = N+1 / `minReaderVersion` = current package (the additive-forward case), + and a synthetic `formatVersion` = N+1 / `minReaderVersion` = N+1 package (the refuse case). + A test that regenerates its own fixture proves only that the code agrees with itself — + which is precisely the failure mode a format ladder exists to catch. +- **A truncation corpus.** The valid package truncated at a spread of offsets, each asserted + `Malformed` rather than `TooNew`, so the two recoveries never get crossed. +- **A hostile-name corpus.** Packages whose entry names carry `..`, separators, and absolute + prefixes, each refused. +- **The round-trip anchor.** Export → import → export over the v1 fixture yields + byte-identical payloads. +- **A standalone DAW verification script** in `docs/`, following the + `docs/verify-track-scope-multitrack.md` precedent — the cross-machine transfer is the one + claim no unit test can make. +- **The corpus is append-only.** When a future format version ships, its fixture is added; + no existing fixture is ever regenerated or edited. Stated in the corpus's own README so the + rule survives the person who wrote it. + +**Acceptance criteria.** +- All three version fixtures classify as specified, and the additive-forward one imports with + every known field intact and every unknown key skipped. +- Every truncation offset classifies `Malformed`; none classifies `TooNew`, `Readable`, or + crashes. +- Every hostile-name fixture is refused at decode, before any planner runs. +- The round-trip fixture's payloads are byte-identical after export → import → export. +- The DAW script exists and names its steps concretely enough to run without reading this + plan. + +**Open questions.** **[propose at review]** how large the committed corpus is allowed to be — +the recommendation is one-sample packages with a few hundred bytes of payload each, since the +properties under test are structural and a large payload proves nothing extra. + +--- + ## Traceability — all seventeen items The check that nothing was dropped. Every row points at a track that exists above. @@ -2303,6 +2811,13 @@ proof it exists to give. plan's sequencing claim. If more corrections of this shape appear, they belong here rather than in the table — the table is a completeness proof over `TODO-1.0.md`, and a correction has no source row to point at. +- **All of Phase Ε** (`pe-*`). **Six tracks across three waves**, from a direct request + (Daniel, 2026-08-02), not from `TODO-1.0.md`. Listed here as a block, like Γ and Ψ; the + product reasoning lives in `docs/product/bank-package.md`. It **supersedes nothing** — + neither `docs/TODO.md` nor `docs/TODO-1.0.md` records export, import, or a package format, + so there is no deferred entry to absorb or contradict. It is also the only phase in this + plan that opens with unanswered [Daniel]-class forks rather than closing them; see + "Decision state" above. ### Deliberate compressions @@ -2415,4 +2930,27 @@ Phase Psi — The extension trust pass (none of the seventeen; a direct capture.cpp + capture_realtime_finalize.cpp (W2-T1 naming lines | W2-T2 channel lines) — all textual adjacency, not semantic contention. main.cpp is W1-T3's exclusively. + +Phase Epsilon — The bank package (none of the seventeen; a direct request 2026-08-02) + W1 The contract, the filesystem, and the ledger's new kind [3 tracks, disjoint by dir] + T1 package-format .............. core/package: framing + TWO version ints + [BLOCKED until E-F1 is ruled — Daniel] + T2 package-fs-shell ............ shell/package: atomic write, streaming, pickers + [no REAPER save-picker exists; SWELL/Win32 split] + T3 import-origin-kind .......... OriginKind::PackageImport = 5, append-only + W2 The two verbs [2 tracks; disjointness CONDITIONAL — see below] + T1 bank-export ................. export_plan + verb + action; project untouched + T2 bank-import ................. import_plan + verb + action + .rsbank drop + [OPEN: E-F2 import target, E-F3 degraded ledger] + W3 The compatibility fixtures [1 track] + T1 package-compat-fixtures ..... frozen bytes prove BOTH version directions + + Version policy, both directions: older package in newer build ALWAYS imports (additive + fields default, unknown keys skipped); newer package in older build REFUSES WHOLE with a + three-part message (package needs / this build reads / writer semver). The gate is + minReaderVersion <= kPackageFormatVersion — formatVersion is for the message, not the gate. + Shared files, named: root CMakeLists.txt add_subdirectory list (W1-T1 | W1-T2, one + append-only line each); main.cpp + the panel bank menu (W2-T1 | W2-T2, one registration + line and one menu row each). W1's three tracks are unconditionally disjoint; W2's two are + textually adjacent only — serialize T2 behind T1 if zero contention is wanted. ``` diff --git a/docs/product/bank-package.md b/docs/product/bank-package.md new file mode 100644 index 0000000..4a0e1b2 --- /dev/null +++ b/docs/product/bank-package.md @@ -0,0 +1,582 @@ +# Bank package — product notes + +Framing, rationale, and design-direction calls behind **Phase Ε — bank export and +import as a single-file package**. The tickable spec lives in `docs/PLAN.md` +(§Phase Ε); the architecture detail will live in `src/core/package/CLAUDE.md` and +`src/shell/package/CLAUDE.md` once those directories exist. This doc holds the +*why* — the user problem, the container choice, the version-compatibility policy +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. + +--- + +## What it is (and what it is not) + +**A bank package is one file that carries one bank — its audio and its index — +out of a project and into another.** Today a bank is per-project by construction: +the audio sits in `/reasampler_bank/` (`core/capture/capture_paths.h:16`, +`kBankSubfolder`) and the index that gives that audio meaning lives in the `.rpp`'s +project ext state under the `"reasampler"` namespace (`src/ext_keys.h:25`, +`kProjExtBanksKey`). The two travel together with the project and nowhere else. +Export writes both halves into a single `.rsbank` file; import lands them into +another project's bank folder and index. + +**It is not a project-transfer feature.** REAPER already moves projects — *Save +project as… with copy of media*, track templates, subprojects. None of them can +carry a ReaSampler bank, because none of them knows the ext-state index exists; +copy the `reasampler_bank/` folder by hand into another project and you get a pile +of `.wav` files with no names, no loop points, no root notes, no tempo stamps, no +tiers, and no lineage. The package exists precisely because **the metadata is the +part that cannot be moved by hand.** + +**It is not a preset.** A package carries audio plus bank metadata. It does not +carry ReaSampler 9000's dialed sound — filter, envelopes, splines, loop crossfade, +rate, pitch. That is the instrument's `ComponentState`, and a user who wants the +dialed sound in another project bakes it first (Phase Ξ's resample) and exports the +resulting capture. The package is a *bank*, and the bank has always been the audio, +not the instrument. See "What a package deliberately does not carry" below — this +is the most likely user expectation mismatch in the whole feature, so it is headed +off here rather than discovered in a support thread. + +**It is not a re-encode.** Sample bytes leave the source project and arrive at the +destination byte-identical. Frame count, sample rate, bit depth, channel count are +untouched; no trim, no normalize, no mono collapse, no format conversion, no +compression of the audio payload. The package payload is **opaque bytes** to +everything in the export/import path except a hash function. This is the phase's +trust anchor, and it is the direct analogue of the capture pillar's null test. + +--- + +## Why a single file, not a folder copy + +The obvious cheap alternative is "copy the bank folder, and write the index into a +sidecar JSON beside it." Rejected, for four reasons, in descending order of weight: + +1. **A folder has no place to put its own manifest that a user cannot lose.** The + index is the part that makes the audio a bank. In a folder, the manifest is just + one more file among two hundred `.wav`s — droppable, renamable, editable into + inconsistency, and silently absent after a partial copy. In a single file it is + the header, and the file either has one or is not a package. +2. **Integrity and version tagging need one identity.** "Is this package complete, + and can this build read it?" is answerable in one read of one file's first few + kilobytes. A folder answers it only after enumerating and stat-ing every entry, + and answers "was anything edited since export?" not at all. +3. **The move gesture is one object.** Email it, drop it in shared storage, drop it + on the docked panel. The panel already accepts `WM_DROPFILES` for ingest + (`src/shell/panel/panel_window.cpp` header comment: "WM_DROPFILES -> ingest"), so + a package can ride an affordance that exists. +4. **Atomicity is buyable.** A single file can be written to a temp path and + atomically renamed on success — the precedent the mono collapse already set + (Ψ-W2-T2 landed the collapse "via temp file plus atomic rename"). A half-written + 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. + +--- + +## The container — three candidates (fork Ε-F1) + +### (a) ZIP, via the vendored minizip + +`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. + +- **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. + +### (b) A hand-rolled `RSBK` container — **recommended** + +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. + +- **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. + +--- + +## Version tagging: two questions, and why one number cannot answer both + +### The precedent this extends (read from source, 2026-08-02) + +The repo already carries **two** versioning mechanisms, and they answer different +questions: + +1. **A blob-schema ladder.** `src/core/tracking/origin_ledger.cpp:8-30` states the + ladder for the `owned_files` blob in a header comment (v1 legacy path-only, v2 + current), pins `constexpr int kLedgerVersion = 2`, and — the load-bearing part — + **reads and validates `"v"`, not merely writes it**: "A version above + `kLedgerVersion` is therefore its own degraded status, never a Loaded ledger" + (`:20-21`). The parse outcome is a three-way `Ok` / `Malformed` / `FutureVersion` + (`:171`, `:185`), deliberately distinguished so the operator gets the right + recovery advice. A *field-vocabulary* gap behaves oppositely: an unrecognized + `OriginKind` integer degrades to `Unknown` rather than failing the parse + (`:32-40`), because "a vocabulary gap must not halt the prune" + (`src/core/tracking/CLAUDE.md:82-86`). +2. **An app writing-version stamp.** `src/core/version/app_version.h:165-186` — + `WritingVersion` with `PreVersioning` / `Unknown` / `Stamped`, classified by + `classifyWritingVersion`, stamped into project ext state by + `src/shell/persist/ext_state_io.cpp:172-176` using `stampVersion()` (the numeric + triple only, no channel suffix). It is informational: an absent stamp is "not an + error and not a warning" (`app_version.h:166-168`). + +**An observation worth recording, not a defect to fix here:** `BankBook` writes +`"version": 1` into the banks blob (`src/core/model/bank_book_json.cpp:37`) but its +parser skips the key along with every other unknown one +(`bank_book_json.cpp:182` — `if (!r.skipValue()) return false; // version, or unknown`). +The book's version field is therefore **decorative today** — written, never read, +never gating. The ledger's is the precedent to extend; the book's is the precedent +not to repeat. + +### The two questions a package must answer + +- **"Can I parse this shape at all?"** — a hard gate. Monotonic integer. This is + `origin_ledger`'s `"v"`. +- **"Who wrote this, so I can tell the user what to open it with?"** — informational, + never a gate. Semver string. This is `app_version`'s stamp. + +A package carries **both**, and conflating them is the mistake to avoid. The stamp +alone cannot gate (semver ordering does not track schema shape; a patch release can +change a blob and a minor release can leave it alone). The ladder alone cannot +advise (an integer tells a user nothing about which build to install). + +### The refinement: `formatVersion` **and** `minReaderVersion` + +A single ladder has one bad property: **every change strands every older reader, +even a purely additive one.** That is not hypothetical here — look at what `Sample` +has actually accumulated: `rootNote` and `loop` (`bank_model.h:112-122`, +"additive like `provenance`. Both default cleanly empty"), `captureTimeSigNum` / +`captureTimeSigDenom` (`:103-108`, "0/0 means UNSTAMPED"), `channelCount` +(`:91-96`, "0 = unknown — a pre-field entry"). Every one of those was additive with +a defined absent-value. Under a single ladder, each would have blocked older readers +for no reason. + +So the package header carries two integers: + +- **`formatVersion`** — what this writer emitted. Monotonic, bumped on any change. +- **`minReaderVersion`** — the oldest reader that can read this package *safely*. + Bumped only when a change is **structural** (a field's meaning changes, a section + is removed, framing changes); left alone when a change is **additive** (a new + optional manifest key, a new enum value with a defined degrade). + +The reader's rule is one line: **read it iff +`minReaderVersion <= kPackageFormatVersion`.** `formatVersion` is then only for the +message text and the log. + +This is a borrowed pattern, not an invention: Matroska's `EBMLVersion` / +`EBMLReadVersion` pair, PDF's catalog `/Version` over the header version, and OOXML's +`mc:Ignorable` markup-compatibility mechanism all separate "what I am" from "what you +must understand to read me." It costs one extra integer and one writer discipline — +*decide honestly whether your change is additive* — and that discipline is exactly +the one `origin_ledger` already enforces on `OriginKind` +(`src/core/tracking/CLAUDE.md:82-83`: "PERSISTED INTEGERS — never renumber, only +append"). + +### Both directions, concretely + +**Direction 1 — newer ReaSampler, older package. Always imports. Never refuses.** +Every reader reads every `minReaderVersion <= kPackageFormatVersion`. Absent manifest +keys take their defined defaults, exactly as `Sample`'s additive fields already do, +and exactly as `origin_ledger` lifts a v1 path-only blob into v2 records with kind +`Unknown` and empty ids (`origin_ledger.cpp:14-16`). Unrecognized manifest keys are +skipped, which is already how every parser in this repo behaves +(`bank_book_json.cpp:182`). Unrecognized enum integers degrade to their defined +`Unknown`-equivalent, never to the numeric default and never to a parse failure — +`bake_wire`'s rule verbatim (`src/core/wire/CLAUDE.md:83`: "an unrecognized value +decodes as `Failed` rather than as the numeric default `Ok`"). +**The user sees:** a normal import summary. Optionally a single console line naming +the older writer version. No dialog, no warning, no ceremony — a supported case is +not an incident. + +**Direction 2 — older ReaSampler, newer package. Refuses. Whole-package, nothing +written.** `minReaderVersion > kPackageFormatVersion` is a hard stop, before a single +byte is written to the bank folder and before the index is touched. This is exactly +`LedgerStatus::FutureVersion`'s treatment, and for the same reason stated at +`origin_ledger.cpp:18-21`: parsing an unknown shape by old rules "would yield a +plausible-but-partial" result, and a partial bank is worse than no bank. +**The user sees** a message box (`ShowMessageBox`, verified — +`vendor/reaper-sdk/sdk/reaper_plugin_functions.h:6546`, +`int (*ShowMessageBox)(const char* msg, const char* title, int type)`) naming three +things, because any two of them leave the user stuck: + +> **Cannot import this bank package.** +> It was written by ReaSampler 1.7.0 and needs package format 3 or newer. +> This build (1.5.2) reads package format 2. +> Nothing was imported. Install ReaSampler 1.7.0 or newer and try again. + +The writer's semver is what makes the message *actionable* — "format 3" alone tells a +user nothing they can act on. That is the whole reason both fields exist. + +**Refusing is the correct direction to refuse in**, and it is worth saying why +rather than leaving it as taste: the destination project is the user's existing work. +A refusal costs a transfer the user can retry after updating. A best-effort partial +import costs silent data absence inside a project they will keep working in, and they +will not find out which twelve of forty samples were dropped until they need one. + +--- + +## What a package carries + +- **The two version fields and the writer's semver**, in the fixed header. +- **An export timestamp** and the **source bank's display name** — informational, and + the default the import prompt pre-fills. +- **One manifest entry per sample**, carrying that `Sample` record in + **`bank_model`'s own serialization, nested verbatim**. This is the + `bank_book_json` precedent applied outward: the book writer "emits the bank + envelope … plus a raw `index` member whose value is the `BankModel` blob verbatim, + so per-bank sample serialization stays owned by `bank_model` and is not duplicated + here" (`bank_book_json.cpp:15-20`). The package does the same, so a future `Sample` + field reaches packages for free and the shape has exactly one owner. +- **Per entry, additionally:** the payload's **bare file name** inside the package, + its byte length, and a whole-file `hashBytes` digest + (`core/capture/wav_codec.h:143` — FNV-1a 64-bit over raw bytes, 16-char lowercase + hex). Note carefully: `hashBytes`, **not** `hashWavContent`. The latter deliberately + skips non-`fmt `/`data` chunks (`wav_codec.h:145-151`), which is right for dedup + identity and wrong for "did these bytes survive the trip." Both hashes are already + in the codebase; the package needs the raw one for integrity and carries the + `Sample`'s existing `contentHash` for dedup, and they are different fields + answering different questions. +- **The bank's slot map** — display positions (`core/model/slot_map`), already JSON + round-trippable. A bank's arrangement is part of what the user built. +- **The payloads**, byte-exact, in manifest order. + +`hashBytes` is FNV-1a — a corruption detector, not a cryptographic checksum. Say so +plainly in the code and in any user-facing wording: it catches truncation, bit rot, +and a mangled transfer. It does not certify provenance, and it is not a defense +against a package deliberately crafted to collide. That is the right level of +guarantee for this feature; overselling it would be the error. + +## What a package deliberately does NOT carry + +- **Any absolute path. Any path at all.** Entries are **bare file names** — no + directory component, no `..`, no drive letter, no leading separator — validated on + encode *and* on decode. The importer spells the destination path itself, through + the same `capture_paths` arithmetic every capture already uses. This makes the + relative-paths-only precision invariant **structural rather than remembered**: + there is no field in the format capable of expressing an absolute path. It also + closes the archive-traversal ("zip slip") bug class by construction, which is the + one genuinely security-shaped surface this feature has. +- **The origin ledger.** The ledger is *this project's* record of files *it* + created, and it is the authority prune's protected set is computed from + (`src/core/tracking/CLAUDE.md:5-13`). Importing foreign ownership records would + assert this project's authority over another project's history. Instead the + importer writes **its own** birth records for the files it lands, at the moment it + lands them, through the one writer (`ReaSamplerSession::recordCreated`, + `src/shell/persist/session.h:95` — it already takes an `OriginKind`). Without that, + every imported file would be "foreign, therefore never reclaimed" + (`core/tracking/CLAUDE.md:24-31`) and a user's bank folder would grow forever. +- **Live-instance usage records** (`rsusage_*`, `src/ext_keys.h:65`). Per-instance + runtime state of a specific project's specific FX instances. Meaningless elsewhere. +- **Project state that is not bank state:** which bank was active, the Design View + mode model (`view_state`), the tail setting, the project GUID, the bank-generation + counter. A package is a bank, not a project. +- **ReaSampler 9000's `ComponentState`.** Stated above; restated here because it is + the expectation most likely to be wrong. The seam is left open, not closed: the + manifest skips unknown keys, so a future `instrumentState` section is a purely + additive change that does not bump `minReaderVersion`. Designing that seam now and + spending it later is the point. + +--- + +## Identity and collision on import + +Four distinct collisions hide under the word "collision," and they need four +different answers. + +1. **Sample id.** Ids are minted as `"cap-" + uniqueTag + "-" + fileName` + (`src/shell/capture/capture.cpp:567`) and `"imp-" + …` + (`src/shell/actions/ingest.cpp:269`) — unique within a project, **not** globally. + Re-importing a package into the project it came from would collide. + **Answer: remint every sample id on import**, under its own prefix, and remap + `Provenance::parentSampleId` (`bank_model.h:45-50`) through the same map — to the + reminted parent if that parent came in the same package, cleared otherwise. A + foreign id never enters the destination index. This also makes "import the same + package twice" a clean, duplicative, correct operation rather than an undefined + one. +2. **File name in the destination bank folder.** **Never overwrite.** Overwriting + would destroy an existing capture, and only prune touches existing bank bytes. + Mint a fresh unique name through the existing `deriveBankPaths` + + unique-tag machinery (`core/capture/capture_paths.h:42`), automatically, no + prompt, and report the count in the summary. +3. **Content hash.** `BankModel::add` collapses an equal-`contentHash` add onto the + existing entry (`bank_model.h:144-147`, `AddResult::Collapsed`). Desirable — but + if the file was already written to disk before the collapse, it becomes an + instant orphan. **Answer: check the destination bank's `findByHash` BEFORE writing + the payload**; on a hit, skip the write entirely and report "N already present." + 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. + +--- + +## Failure modes and what the user sees + +Whole-package, all-or-nothing on both sides. The reasoning is the same one prune +settled on: report before acting, and never leave a half-state that looks whole. + +| Failure | Side | Behaviour | What the user sees | +|---|---|---|---| +| An indexed file is missing on disk | export | Refuse by default; offer "export the N present entries" only behind an explicit confirm that lists what is missing | Message box naming the missing entries; nothing written unless confirmed | +| An indexed file is unreadable (locked/permission) | export | Same as missing | Same, distinguishing unreadable from absent | +| Destination package file exists | export | Platform save dialog's own overwrite confirm | Native dialog | +| Write fails partway | export | Temp file in the destination directory, atomic rename only on complete success | Console error; no `.rsbank` left behind. A truncated package must never exist | +| `minReaderVersion` above this build | import | Refuse whole. Nothing written, index untouched | The three-part message box above (package needs / this build reads / what to install) | +| Malformed or truncated container | import | Refuse whole. Reported **distinctly from** the version case | "This file is not a readable bank package (corrupt or truncated)." The distinction matters: the two have opposite recoveries — one is "install a newer build," the other is "get an intact copy." `origin_ledger.cpp:178-185` makes exactly this distinction for exactly this reason | +| 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 | +| 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 | + +**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 — +`src/shell/persist/prune_fs.cpp:5-11`: "a shell removing a file it wrote itself +moments earlier and that no index ever referenced is self-cleanup, not authority +over user data … the discriminator is 'did this call create it, and did anything ever +reference it', not where it sits." An import rollback fits that discriminator +exactly: the files were written by this call, and the index mutation is abandoned, so +nothing ever referenced them. The spec must **cite** the carve-out rather than +restate it, or a reviewer will correctly read the rollback as a breach. + +**On undo.** The index side of an import is one Ctrl-Z, through the same +`persistBankOp` undo batching every bank verb already uses +(`src/shell/bank_ops/CLAUDE.md:29-31`; `Undo_BeginBlock2` / `Undo_EndBlock2` verified +at `reaper_plugin_functions.h:7758` and `:7806`). **Undo does not un-write the +files** — they remain on disk, referenced by no index, until a prune reclaims them. +That is the same designed orphaned-until-prune window a non-empty bank delete already +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. + +--- + +## Memory: the streaming seam that keeps the codec pure + +A bank is not small. Float32 stereo at 48 kHz is ~23 MB per minute; a two-hundred- +sample bank is plausibly gigabytes. **The naive shape — a pure +`encodePackage(vector) -> vector` — holds the whole bank twice in +RAM and is unshippable.** The temptation is then to move the codec into the shell so +it can stream. That is the wrong correction, and the right one is a better seam: + +- **Pure owns framing and arithmetic.** `encodeHeader(manifest) -> bytes` and + `entryLayout(manifest) -> [{ name, offset, length }]` on the write side; + `decodeHeader(prefix bytes) -> manifest + entry layout` on the read side. Offsets + and lengths are arithmetic — perfectly pure, perfectly testable, and the exact + place an off-by-one becomes a corrupt package. +- **Shell owns the stream.** It writes the header, then appends payloads one at a + time, reading each source file into a buffer, hashing it, writing it, and releasing + it. On decode it reads the prefix, gets the layout, then seeks and streams each + payload independently. + +**Constraint, stated as an acceptance criterion:** the export and import paths hold +**at most one entry's payload** in memory at a time. This is what keeps the codec +pure without making the feature fail on real banks, and it is the kind of thing that +is cheap to design in and expensive to retrofit. + +**One honest cost.** Export and import are synchronous, on the UI thread, like every +other action in the tool, and prune sets that precedent (a scan-then-confirm gesture +that blocks). A multi-gigabyte bank will therefore freeze REAPER for seconds. The +recommendation is to ship synchronous with a console progress/summary line and treat +async as a later move if it bites — but this is a real `[propose]`-class call the +implementation review should make deliberately rather than by default. + +--- + +## Where it lives (pure / shell) + +Two new directories, following the split the whole repo turns on. + +**`src/core/package/` — pure, REAPER-free, unit-tested without a DAW.** + +- `package_format` — the container framing and the version ladder in one place: + the magic, the header layout, `kPackageFormatVersion`, `kPackageMinReaderVersion`, + and `classifyPackageVersion(formatVersion, minReader) -> Readable | TooNew | + Malformed`. The ladder lives with the framing because the ladder *is* the framing's + contract, and it gets a header-comment ladder written the way + `origin_ledger.cpp:8-21` writes one. +- `package_manifest` — the manifest model and its JSON codec, nesting `BankModel`'s + own blob verbatim. +- `bank_package` — header encode / prefix decode / entry layout, composing the two + above. Never holds a payload. +- `export_plan` — the pure export decision: which entries, what names, what is + missing, and therefore whether the export may proceed. +- `import_plan` — the pure import decision: the id remap table, the parent remap, the + per-entry write / skip-already-present / rename-to-avoid-collision disposition, and + the destination bank name after uniqueness folding. **This module is why the whole + feature is testable without a DAW** — every collision rule above is a pure function + over strings and hashes. + +`export_plan` and `import_plan` are separate TUs deliberately, not one `package_plan`: +they share only the manifest type, and separating them is what lets the two Phase Ε +build tracks run in parallel without fighting over a file. The seam is a +responsibility seam, which is what the structural heuristic asks for. + +**`src/shell/package/` — filesystem and REAPER-facing.** + +- `package_io` — read a package file to bytes, write bytes through temp + atomic + rename, read a bank file's bytes, write a landed file, enumerate existing bank-folder + names, and execute the rollback delete (citing the `prune_fs` carve-out). +- The platform file pickers, under the `#ifdef _WIN32` / `#else swell/swell.h` split + this codebase already uses (`src/shell/panel/draw_kit.cpp:11-15`, + `src/shell/persist/prune_fs.cpp:35-38`). **Verified:** the REAPER API offers a *read* + picker — `GetUserFileNameForRead(char* filenameNeed4096, const char* title, const + char* defext)`, `reaper_plugin_functions.h:3798` — and **no save picker at all** + (a sweep of the header for `FileNameFor|SaveFile|Browse` returns only that one + entry). Export's destination picker therefore comes from Win32 `GetSaveFileNameW` + on Windows and SWELL's `BrowseForSaveFile` elsewhere + (`vendor/WDL/WDL/swell/swell-functions.h:167`). This is a real asymmetry between + the two verbs and the spec should not paper over it. +- `export_bank` / `import_bank` — the promptless verbs, mirroring + `src/shell/bank_ops/`'s pattern exactly: take a `ReaSamplerSession&`, do the work, + return an outcome, **no prompts and no message boxes**. The bindable action and the + panel menu item are then thin skins over one verb apiece, so the logic has one home + (`src/shell/bank_ops/CLAUDE.md:1-12`). + +**The dependency-shape criterion, stated because the brief demands it.** The pure +planners take **explicit value inputs** — the decoded manifest, the destination +`BankBook`, the set of file names present in the bank folder — never a session handle, +never a service container, never a "pass me the thing that has everything." The shell +*gathers*; the core *decides*. That is the same shape `src/shell/persist/CLAUDE.md:11` +already states ("it gathers rather than decides"). If a circular dependency shows up +during the build, the fix is a service split or a thin interface at the seam — never +threading an extra parameter through a chain of constructors, and never handing a +container down. A base class that grows a dependency must not grow its subclasses' +constructors. + +--- + +## Invariant reconciliation + +- **Relative paths only.** Strengthened, not merely preserved: the package format has + no field capable of expressing a path, only a bare file name, validated at both + ends. The destination path is spelled by `capture_paths` on the importing side. +- **Capture and placement are separate acts.** Import writes files and index entries. + It places **no** timeline item, ever — the same rule capture has always carried + (root `CLAUDE.md`, "The load-bearing principle"). A user who wants the imported + audio in the arrange uses the existing insert action. +- **Prune is the single exclusive file-deletion authority.** Unchanged. The one + rollback path is the documented self-cleanup carve-out, cited not restated. +- **No lossy transforms.** The payload is opaque bytes on both sides. `wav_codec` is + invoked on it only to hash and to read metadata already recorded — never to rebuild, + trim, normalize, or collapse. The mono collapse in particular is a **capture-path** + behaviour and must not reach the import path, for the same reason ingest is already + excluded from it (root `CLAUDE.md`, exact-bounds invariant: "ingest is excluded, + because an imported file is the user's bytes, not our capture"). A package's bytes + are someone else's capture; the same exclusion applies with the same reasoning. +- **Bit-identical round-trip.** Export → import → export yields byte-identical + payloads. This is the phase's trust anchor and belongs in the acceptance criteria of + the round-trip track, tested against frozen fixture bytes rather than against a + freshly-generated pair. +- **Bank generation.** Import mutates bank content that live ReaSampler 9000 + instances may play, so it must `bumpBankGeneration()` + (`src/shell/persist/session.h:108`, whose own comment says call sites "err toward + bumping"). Export mutates nothing and must bump nothing, write no ext state, and + open no undo point. +- **Beta/stable channel isolation.** Packages are channel-**agnostic** and this is + deliberate. Channel isolation exists so a beta cannot rewrite a stable project's + ext state (`app_version.h:73-76`); a package is a file the user moves by hand, not + ambient project state, so there is no isolation property to preserve. A beta build + and a stable build at the same package format read each other's packages, and that + is the useful behaviour. The version ladder — not the channel — is what gates. + +--- + +## Open forks — Daniel's + +**Ε-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. + +**Ε-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. + +**Ε-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. + +--- + +## Non-goals and guardrails + +- **No auto-insertion of imported audio into the arrange.** Same rule as capture. +- **No overwrite of an existing bank-folder file, ever.** Auto-rename instead. +- **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 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 + is the unit users think in. A future multi-bank package is an additive manifest + change that does **not** bump `minReaderVersion`, so the option is preserved by + construction rather than by promise. Do not build it now. +- **Do not make the package a sync mechanism.** No "re-import to update," no + reconciliation against a previously-imported package, no package identity tracked + in project state. Import is a one-way copy-in. Anything else is a different product.