86c3c7f3b8
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.
583 lines
36 KiB
Markdown
583 lines
36 KiB
Markdown
# 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 `<projectDir>/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 `<name>` 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<uint8_t>) -> vector<uint8_t>` — 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.
|