Merge Phase Ε: the .rsbank bank package — pure codec, filesystem shell, both verbs, and the frozen compatibility corpus
# Conflicts: # docs/COMPLETED.md
This commit is contained in:
@@ -981,3 +981,138 @@ remains empirical, not proven; and `TailMode::Auto`/`Manual` have no automatic b
|
||||
observation at all — `render_bounds_gate` judges `TailMode::None` only, so both modes are
|
||||
fixed by inference (same bounds path, same floor) rather than by measurement, and only
|
||||
the 0-byte gate covers them until a DAW check closes it.
|
||||
|
||||
### Ε-W1 — The contract, the filesystem, and the ledger's new kind
|
||||
|
||||
Phase Ε's first wave: the `.rsbank` package contract, the filesystem/dialog seam
|
||||
behind it, and a new tracking-ledger origin kind for package-sourced files — three
|
||||
tracks, disjoint by directory, dispatched in parallel.
|
||||
|
||||
**Ε-W1-T1 — `package-format`.** The pure `src/core/package/` codec for the
|
||||
hand-rolled `RSBK` container (Ε-F1, ruled — no ZIP, no compressor, no link edge to
|
||||
`vendor/WDL/WDL/zlib/`): a fixed little-endian header carrying two version
|
||||
integers — `formatVersion` (what the writer emitted) and `minReaderVersion` (the
|
||||
oldest reader that can read it safely) — a length-prefixed JSON manifest, and
|
||||
payloads concatenated in manifest order. `classifyPackageVersion` answers
|
||||
`Readable`/`TooNew`/`Malformed`; a `TooNew` header refuses whole, producing no
|
||||
manifest, so the refusal can still name the writer's semver rather than
|
||||
half-succeeding. Landed as three modules: `package_format` (the contract, the
|
||||
version ladder, and three name-validation rules — `isValidEntryName`,
|
||||
`sameEntryName`'s ASCII-case fold, `isValidNestedSamplePath`), `package_manifest`
|
||||
(the manifest model + JSON codec, carrying the bank's `slot_map` and a whole-file
|
||||
`hashBytes` digest per entry — deliberately not `hashWavContent`, which skips
|
||||
chunks and so cannot answer "did these bytes survive"), and `bank_package`
|
||||
(framing/layout arithmetic: `encodePackage`/`decodePackage`/`requiredPrefixSize`,
|
||||
never holding or hashing a payload itself). Hostile input is refused, never UB,
|
||||
at every byte offset.
|
||||
|
||||
**Ε-W1-T2 — `package-fs-shell`.** `src/shell/package/`: streaming, atomic package
|
||||
filesystem I/O (`package_io`'s `PackageFileWriter`/`PackageFileReader`, at most one
|
||||
entry's payload materialized at a time, backed by a `.rsbanktmp` sibling that
|
||||
reaches the destination only through a `commit()` rename — process-crash atomic,
|
||||
not power-loss atomic, deliberately, since an `fsync` over a whole sample bank is a
|
||||
real stall) and the rollback journal (`package_rollback`'s `LandedFileJournal`,
|
||||
citing the `prune_fs.cpp` carve-out rather than restating it, disarmed only after
|
||||
the caller's own write has returned success). `package_pickers` rides REAPER's own
|
||||
`GetUserFileName` for both directions, as specified (mode 1 import, mode 0 export)
|
||||
— the plan's "REAPER has no save picker" finding was a regex miss in the original
|
||||
research, not a real gap, so there was no asymmetric-picker deviation to land: no
|
||||
SWELL `BrowseForSaveFile`, no Win32 `GetSaveFileNameW`, no `GetUserFileNameForRead`
|
||||
(the SDK header marks it superseded). REAPER owning the dialog on every platform is
|
||||
why there's no platform split; that's separate from `main.cpp` already aborting
|
||||
extension load if any needed API pointer fails to resolve, which is why no fallback
|
||||
path is needed. Both pickers are `[verify — DAW]`, never exercised in a live REAPER
|
||||
session.
|
||||
|
||||
**Ε-W1-T3 — `import-origin-kind`.** `OriginKind::PackageImport` appended to the
|
||||
tracking ledger as value 5 — package-sourced vs `Ingest`'s user-picked. Append-only,
|
||||
per `core/tracking/CLAUDE.md`'s persisted-integer rule; an unrecognized kind
|
||||
degrades to `Unknown` rather than failing the parse, and `kLedgerVersion` stays at
|
||||
2 — a vocabulary addition, not a document-version bump. No decision surface
|
||||
changed: `pruneProtection`'s output is unaffected for every existing kind.
|
||||
|
||||
### Ε-W2 — The two verbs
|
||||
|
||||
Two tracks landed on Ε-W1's contract: a bank leaves the project as one `.rsbank`
|
||||
file, or the export refuses and says why; a `.rsbank` becomes a **new** bank,
|
||||
completely or not at all. Both tracks were code-reviewed and remediated before
|
||||
merging; the merged tree (Ε-W1 + Ε-W2) builds clean and passes 100/100 tests.
|
||||
|
||||
**Ε-W2-T1 — `bank-export`.** New `core/package/export_plan` (pure: which entries,
|
||||
what names, what is missing, and therefore whether the export may proceed — verdict
|
||||
`Ready`/`Incomplete`/`Refused`) and `shell/package/export_bank` (the promptless
|
||||
verb, in three composable public steps — `surveyBankExport`, `digestSources`,
|
||||
`writePackageFile` — arriving with a **const** `ReaSamplerSession&`, so "writes no
|
||||
ext state, opens no undo point, never bumps the generation" holds by the type
|
||||
rather than by memory), plus `shell/actions/package_export_action`, one
|
||||
`main.cpp` action-table row, and one panel bank-menu row. Nothing is re-encoded;
|
||||
payloads are copied and hashed. The exported unit is one bank — the pool included,
|
||||
since the pool is structurally one `BankIndex` among many — and whole-book export
|
||||
stays out of scope for the phase. Both open questions were answered at review:
|
||||
affordance ships as **both** the bindable action and the panel row, and the
|
||||
default file name derives from the bank's display name through
|
||||
`capture_paths::sanitizeStem`.
|
||||
|
||||
**Ε-W2-T2 — `bank-import`.** New `core/package/import_plan` (pure: the id remap
|
||||
table, the parent remap, the per-entry land/skip-already-present/rename
|
||||
disposition, and the destination bank's display name after `BankBook`'s own
|
||||
uniqueness fold — reached through a new additive `BankBook::uniqueDisplayName`
|
||||
member, the only `core/model/` edit in the phase), and on the shell side a
|
||||
REAPER-free `import_landing` (decode, verify every payload's `hashBytes` digest
|
||||
against the manifest BEFORE the bank folder is created, then land through the
|
||||
rollback journal) plus a REAPER-facing `import_bank` (the only piece touching the
|
||||
extension's project state — the undo-batched persist and the generation bump),
|
||||
`shell/actions/package_import_action`, the panel's `.rsbank` drop route, one
|
||||
`main.cpp` row, one panel menu row, and a new `src/core/util/ascii_ws.h`. The
|
||||
tracking-ledger guard runs before the file picker opens (Ε-F3, ruled: refuse
|
||||
outright on `Unreadable`/`FutureVersion`, no confirm-and-proceed); the version
|
||||
gate runs before any byte is written; all four collision classes — sample id, file
|
||||
name, content hash, bank display name — are answered explicitly, with the
|
||||
display-name collision auto-suffixed and never prompted (Ε-F2, ruled: always a new
|
||||
bank, never a merge); birth records land via
|
||||
`recordCreated(sample, OriginKind::PackageImport)` in the same straight-line block
|
||||
as the index add; the index mutation is one Ctrl-Z, and the landed files'
|
||||
survival as orphans until the next prune is stated in the user-facing summary, not
|
||||
left implicit. **Beyond spec:** `import_plan`'s `spelledLikeABankFile` mints a
|
||||
fresh name even absent a collision, whenever the package's own entry name isn't
|
||||
spelled the way `deriveBankPaths` would spell it — counted separately from a
|
||||
genuine folder-name collision (`sanitizeRenameCount` vs `collisionRenameCount`) so
|
||||
a hostile or foreign-spelled entry name (e.g. an unexpected extension) always
|
||||
lands sanitized rather than verbatim.
|
||||
|
||||
### Ε-W3 — The compatibility fixtures
|
||||
|
||||
The phase's third and final wave, and with it Phase Ε's implementation is complete: the
|
||||
version-compatibility policy stated in `docs/product/bank-package.md` is now a property
|
||||
proven against frozen bytes rather than an assertion in a doc.
|
||||
|
||||
**Ε-W3-T1 — `package-compat-fixtures`.** A new checked-in corpus of 23 frozen `.rsbank`
|
||||
fixtures under `tests/fixtures/package_compat/` — one v1 package written by the shipping
|
||||
build (`1.4.0`), a synthetic additive-forward package (`formatVersion` 2 /
|
||||
`minReaderVersion` 1) carrying three keys this build has never heard of, a synthetic
|
||||
structural-refusal package (2/2), nine truncations (one per distinct decode failure
|
||||
site, including one cut at `additive_forward.rsbank`'s own payload boundary), and eleven
|
||||
hostile-name packages (six bad entry names, five bad nested `relativePath` values) —
|
||||
every payload a single 300-byte 16-bit mono WAV, ~15 KB for the whole corpus. Two new
|
||||
test targets decode and exercise it: `package_compat_tests` (frozen bytes decode to
|
||||
exactly what the shipping build wrote, the additive fixture reads with every unknown key
|
||||
skipped, every truncation classifies `Malformed` and never `TooNew`, every hostile name
|
||||
is refused before any planner runs) and `package_round_trip_tests` (the same corpus
|
||||
driven through the actual verbs — export → import → export over `v1_shipping.rsbank`
|
||||
yields byte-identical payloads, and every refusal fixture refuses the whole import with
|
||||
nothing landed and nothing in the index). A new repo-root `.gitattributes` (`*.rsbank
|
||||
binary`) is load-bearing, not decoration: under `core.autocrlf = true`, git's NUL-sniffing
|
||||
heuristic would text-classify a future short, ASCII-heavy fixture and CRLF-mangle it on a
|
||||
Windows checkout, silently breaking the frozen-bytes premise the whole corpus rests on. A
|
||||
standalone DAW verification script, `docs/verify-package-transfer.md`, covers the one
|
||||
claim no unit test can make — a real cross-machine transfer, including the too-new
|
||||
refusal, the truncated-download refusal, and mid-payload corruption, each read off as an
|
||||
exact message string. **Open question resolved:** the recommendation (one-sample
|
||||
packages, a few hundred bytes of payload each) was followed — the corpus holds
|
||||
one-sample packages with a 300-byte payload each. **Deviation from spec:** the plan
|
||||
called for a truncation cut mid-layout; RSBK stores no layout section (the layout is
|
||||
derived from the manifest's entries, not stored as its own section), so the fixture that
|
||||
exercises "the manifest parses, the layout computes, the exact-size proof fails" lands at
|
||||
the payload boundary instead. No production module was touched — the wave adds test-tree
|
||||
files, the corpus, its README, the verification script, and one path variable in the root
|
||||
`CMakeLists.txt`.
|
||||
|
||||
+109
-416
@@ -2366,7 +2366,7 @@ These bind every track in this phase, in addition to the plan-wide set above.
|
||||
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
|
||||
(`src/shell/persist/session.h:121`) 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
|
||||
@@ -2396,15 +2396,25 @@ new directories** (`src/core/package/`, `src/shell/package/`) that no other phas
|
||||
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), and
|
||||
bank menu (W2-T1 and W2-T2, one registration line and one menu row each),
|
||||
`core/model/bank_book.{h,cpp}` (W2-T2 only — **one additive public `const` member**, required
|
||||
by the Ε-F2 auto-suffix rule so the name fold keeps its single home). **No Ε track
|
||||
touches `core/instrument/`, `shell/instrument/`, or any capture backend.**
|
||||
by the Ε-F2 auto-suffix rule so the name fold keeps its single home), and
|
||||
`shell/persist/session.h` (W2-T2 only — one additive public accessor, `ledgerStatus()`, so the
|
||||
import gate can key on `LedgerStatus` alone without going through `pruneDryRun()`'s
|
||||
enumeration+scan). **No Ε track touches `core/instrument/`, `shell/instrument/`, or any
|
||||
capture backend.**
|
||||
|
||||
**All three waves have landed — Phase Ε is complete.** W1 through W3 each carry their own
|
||||
landed note below; see `docs/COMPLETED.md` for every track's full narrative.
|
||||
|
||||
---
|
||||
|
||||
### Ε-W1 — The contract, the filesystem, and the ledger's new kind
|
||||
|
||||
**All three tracks have landed** — Ε-W1-T1 (`package-format`), Ε-W1-T2
|
||||
(`package-fs-shell`), and Ε-W1-T3 (`import-origin-kind`) — see `docs/COMPLETED.md` for
|
||||
the full narrative of each.
|
||||
|
||||
**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.
|
||||
@@ -2427,212 +2437,63 @@ 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.**
|
||||
- **The container is the hand-rolled `RSBK` (Ε-F1, ruled).** 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. Framing is built on `core/wire/bytes.h`
|
||||
(`putLE` / `ByteReader`) and the manifest on `core/json` — both already owned and tested
|
||||
here. **No ZIP, no compressor, no new third-party source in the build**; a link edge to
|
||||
`vendor/WDL/WDL/zlib/` means the ruling was misread.
|
||||
- **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 `Sample` field with a defined absent-value) bumps `formatVersion`
|
||||
only; a **structural** change bumps both. **Growing a persisted enum's vocabulary is
|
||||
structural here, not additive** — `BankModel::deserialize` *rejects* an out-of-range
|
||||
`SourceMode` or `Tier` rather than degrading it (`bank_model.cpp:232-239`, `:339-346`),
|
||||
and every enum a package carries rides inside the nested `BankModel` blob, so a new
|
||||
`SourceMode` or `Tier` value bumps both integers. 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 — the manifest's own, not `BankModel`'s nested ones, which
|
||||
reject per the bullet above — 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.** **No [Daniel] questions — Ε-F1 is RULED** (proprietary `RSBK`), so this
|
||||
track is dispatchable as written. **[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.
|
||||
**Landed** — see `docs/COMPLETED.md` for the full narrative. The pure `src/core/package/`
|
||||
codec for the hand-rolled `RSBK` container (Ε-F1, ruled — no ZIP, no compressor, no link
|
||||
edge to `vendor/WDL/WDL/zlib/`): a fixed little-endian header carrying two version
|
||||
integers — `formatVersion` (what the writer emitted) and `minReaderVersion` (the oldest
|
||||
reader that can read it safely) — a length-prefixed JSON manifest, and payloads
|
||||
concatenated in manifest order. `classifyPackageVersion` answers `Readable`/`TooNew`/
|
||||
`Malformed`; a `TooNew` header refuses whole, producing no manifest, so the refusal can
|
||||
still name the writer's semver rather than half-succeeding. The ladder's one
|
||||
counterintuitive rule rides with the contract: **growing a persisted enum's vocabulary is
|
||||
structural, not additive** — `BankModel::deserialize` *rejects* an out-of-range `SourceMode`
|
||||
or `Tier` rather than degrading it (`bank_model.cpp:232-239`, `:339-346`), and every enum a
|
||||
package carries rides inside the nested `BankModel` blob, so a new value bumps both
|
||||
integers, where a new `Sample` field with a defined absent-value bumps `formatVersion`
|
||||
alone. Landed as three modules: `package_format` (the contract, the version ladder, and
|
||||
three name-validation rules —
|
||||
`isValidEntryName`, `sameEntryName`'s ASCII-case fold, `isValidNestedSamplePath`),
|
||||
`package_manifest` (the manifest model + JSON codec, carrying the bank's `slot_map` and a
|
||||
whole-file `hashBytes` digest per entry — deliberately not `hashWavContent`, which skips
|
||||
chunks and so cannot answer "did these bytes survive"), and `bank_package`
|
||||
(framing/layout arithmetic: `encodePackage`/`decodePackage`/`requiredPrefixSize`, never
|
||||
holding or hashing a payload itself). Hostile input is refused, never UB, at every byte
|
||||
offset.
|
||||
|
||||
#### Ε-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 file-picker seam for both verbs —
|
||||
**one picker, REAPER's own, on every platform**, so this track carries **no**
|
||||
`#ifdef _WIN32` / `#else swell/swell.h` split. That split is a real pattern in this codebase
|
||||
(`src/shell/panel/draw_kit.cpp:11-15`, `src/shell/persist/prune_fs.cpp:35-38`); it is simply
|
||||
not this track's shape, because REAPER owns the dialog. Also owns 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.
|
||||
- **Both pickers are REAPER's own, and they are symmetric.** `GetUserFileName(int mode,
|
||||
const char* caption, const char* initial_file_or_path, const char* extension_list, char*
|
||||
fnOutNeedBig, int fnOutNeedBig_sz)` — **verified**,
|
||||
`vendor/reaper-sdk/sdk/reaper_plugin_functions.h:3790`, documented at `:3788` — serves
|
||||
both directions: `mode=0` ("choose a new file") is export's destination picker, `mode=1`
|
||||
("existing file") is import's source picker. `extension_list` takes the
|
||||
`'ReaSampler banks|*.rsbank|All files|*.*'` form, and `initial_file_or_path` may be a bare
|
||||
`'.rsbank'` to set the default extension. There is **no `#ifdef _WIN32` / SWELL split and
|
||||
no wide-char round trip here** — REAPER owns the dialog on every platform, so no
|
||||
`GetSaveFileNameW` and no `BrowseForSaveFile`. `GetUserFileNameForRead` is explicitly
|
||||
"Superseded, see GetUserFileName" (`:3796`) and is not used.
|
||||
- **No fallback path.** `src/app/main.cpp:15` defines `REAPERAPI_IMPLEMENT` *without*
|
||||
`REAPERAPI_MINIMAL`, so the resolver walks the full table — `GetUserFileName` included
|
||||
(`reaper_plugin_functions.h:9084`) — and `main.cpp:292-293` aborts the extension load if
|
||||
any single function fails to resolve. No REAPER build that loads this extension can lack
|
||||
`GetUserFileName`, which makes a fallback unreachable code.
|
||||
- **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.
|
||||
- **There is exactly one picker call site and it is REAPER's.** Both verbs reach
|
||||
`GetUserFileName` — export with `mode=0`, import with `mode=1` — and no symbol named
|
||||
`GetSaveFileNameW`, `BrowseForSaveFile`, or `GetUserFileNameForRead` appears anywhere in
|
||||
`src/shell/package/`, nor any platform `#ifdef` in the picker's TU. Greppable, so it stays
|
||||
true. `[verify — DAW]` — the picker is not exercised in a live REAPER session by this track.
|
||||
- No file exceeds ~600 lines; the picker lives in its own TU with its own header. **Not** for
|
||||
the `drag_out` / `drag_out_win` reason — that precedent isolates a *Win32-only* TU, and
|
||||
there is no platform split here — but because the picker is the only REAPER-facing part of
|
||||
an otherwise REAPER-free, unit-tested module: folding it into a shared header would drag
|
||||
`reaper_plugin_functions.h` into the testable seam's include graph.
|
||||
|
||||
**Open questions.** **[propose at review]** where the `extension_list` and default-extension
|
||||
strings live — this track's picker TU, or the Ε-W2 verbs that call it. They are user-facing
|
||||
text, and the verbs own the rest of the user-facing text; the counter-argument is that they
|
||||
are picker plumbing and only one picker exists. **[verify — DAW]** two things the header does
|
||||
not answer: (1) whether the `mode=0` picker **appends** an extension when the user types a
|
||||
bare name — `:3788` documents that `initial_file_or_path` may be `'.rsbank'` "to set the
|
||||
default extension," but not that the dialog enforces it on return, so the verb may still have
|
||||
to append `.rsbank` itself; (2) **dialog parenting** — `GetUserFileName`'s signature
|
||||
(`:3790`) takes no owner window, where the abandoned Win32 `OPENFILENAME` path would have
|
||||
passed `GetMainHwnd()`, so modality against the REAPER main window is unobserved.
|
||||
**Landed** — see `docs/COMPLETED.md` for the full narrative. `src/shell/package/`:
|
||||
streaming, atomic package filesystem I/O (`package_io`'s `PackageFileWriter`/
|
||||
`PackageFileReader`, at most one entry's payload materialized at a time, backed by a
|
||||
`.rsbanktmp` sibling that reaches the destination only through a `commit()` rename —
|
||||
process-crash atomic, not power-loss atomic, deliberately, since an `fsync` over a whole
|
||||
sample bank is a real stall) and the rollback journal (`package_rollback`'s
|
||||
`LandedFileJournal`, citing the `prune_fs.cpp` carve-out rather than restating it,
|
||||
disarmed only after the caller's own write has returned success). `package_pickers` is the
|
||||
one picker seam, REAPER's own in both directions as specified — `GetUserFileName` with
|
||||
`mode=0` for export's destination and `mode=1` for import's source, so no platform `#ifdef`,
|
||||
no SWELL `BrowseForSaveFile`, no Win32 `GetSaveFileNameW`, and no `GetUserFileNameForRead`,
|
||||
which the SDK header marks superseded by `GetUserFileName`. No fallback path was needed
|
||||
either: `main.cpp` aborts the extension load if any API pointer fails to resolve, so no
|
||||
REAPER build that loads the extension can lack it. The picker is `[verify — DAW]` in both
|
||||
directions, never exercised in a live REAPER session.
|
||||
|
||||
#### Ε-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.
|
||||
**Landed** — see `docs/COMPLETED.md` for the full narrative. `OriginKind::PackageImport`
|
||||
appended to the tracking ledger as value 5 — package-sourced vs `Ingest`'s user-picked.
|
||||
Append-only, per `core/tracking/CLAUDE.md`'s persisted-integer rule; an unrecognized kind
|
||||
degrades to `Unknown` rather than failing the parse, and `kLedgerVersion` stays at 2 — a
|
||||
vocabulary addition, not a document-version bump. No decision surface changed:
|
||||
`pruneProtection`'s output is unaffected for every existing kind.
|
||||
|
||||
---
|
||||
|
||||
### Ε-W2 — The two verbs
|
||||
|
||||
**Both tracks have landed** — Ε-W2-T1 (`bank-export`) and Ε-W2-T2 (`bank-import`) — see
|
||||
`docs/COMPLETED.md` for the full narrative of each.
|
||||
|
||||
**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.
|
||||
@@ -2657,180 +2518,47 @@ 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.
|
||||
**Landed** — see `docs/COMPLETED.md` for the full narrative. New `core/package/export_plan`
|
||||
(pure: which entries, what names, what is missing, and therefore whether the export may
|
||||
proceed — verdict `Ready`/`Incomplete`/`Refused`) and `shell/package/export_bank` (the
|
||||
promptless verb, in three composable public steps — `surveyBankExport`, `digestSources`,
|
||||
`writePackageFile` — arriving with a **const** `ReaSamplerSession&`, so "writes no ext
|
||||
state, opens no undo point, never bumps the generation" holds by the type rather than by
|
||||
memory), plus `shell/actions/package_export_action`, one `main.cpp` action-table row, and
|
||||
one panel bank-menu row. Nothing is re-encoded; payloads are copied and hashed. The
|
||||
exported unit is one bank — the pool included, since the pool is structurally one
|
||||
`BankIndex` among many — and whole-book export stays out of scope for the phase. Both open
|
||||
questions were answered at review: affordance ships as **both** the bindable action and
|
||||
the panel row, and the default file name derives from the bank's display name through
|
||||
`capture_paths::sanitizeStem`.
|
||||
|
||||
#### Ε-W2-T2 — `bank-import`
|
||||
|
||||
**Goal.** A package becomes a **new** 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" (including the
|
||||
auto-suffix rule), §"Failure modes" (import rows), §"Import under a degraded tracking
|
||||
ledger", §"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, and — the **only** `core/model/`
|
||||
edit in the phase — **one additive public `const` member on `BankBook`** (recommended
|
||||
`std::string uniqueDisplayName(const std::string& seed) const`), so the auto-suffix probe runs
|
||||
behind the model's own name fold. **Does not own:** anything on the export side,
|
||||
`bank_book`'s *rules* (consumed, never re-implemented — the new member exposes the existing
|
||||
fold, it does not add a second one), `origin_ledger` (Ε-W1-T3's).
|
||||
|
||||
**Behavior.**
|
||||
- **The ledger guard runs FIRST — before the file picker opens (Ε-F3, ruled: refuse).** If
|
||||
`tracking::ledgerDegraded(status)` holds for the project's loaded ledger status
|
||||
(`Unreadable` or `FutureVersion`; `core/tracking/origin_ledger.h:94`, `:100-101`), the
|
||||
import **refuses outright** — no picker, no bytes read, no confirm-and-proceed path, no
|
||||
opt-out. `Fresh` and `Loaded` both proceed. **Do not key this on
|
||||
`PruneReport::blockedByTracking`**: that flag also fires on undecodable `rsusage_*` keys,
|
||||
which govern deletion-time protection and have nothing to do with writing birth records.
|
||||
The refusal is a `ShowConsoleMsg` block mirroring `prune_action.cpp:30-69` in structure and
|
||||
tone, with two variants (malformed / newer-build) and every recovery line naming **this
|
||||
build's** namespace through `version::extStateNamespace()`. Exact wording in the spec doc.
|
||||
**Export is deliberately not gated this way** — that is Ε-W2-T1's, and it stays ungated.
|
||||
- **Version gate second, 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** — **auto-suffix, no prompt** (Ε-F2, ruled). Seed = the package's recorded
|
||||
source bank name **verbatim** (or the literal `Imported bank` if absent/blank); take the
|
||||
first of `seed`, `seed + " 2"`, `seed + " 3"`, … whose fold is free in the destination
|
||||
book, ascending from 2. Four points that decide the behaviour and must not be re-invented:
|
||||
the seed is **never re-parsed** (`"Drums 2"` colliding lands as `"Drums 2 2"`, not
|
||||
`"Drums 3"` — a bare trailing integer is indistinguishable from `"Kit 808"`); the probe
|
||||
**fills gaps** (first-free, not highest-plus-one, so it is a pure function of the current
|
||||
name set); the probe **terminates** by pigeonhole within `B + 1` candidates for `B` banks,
|
||||
so **no arbitrary cap**; and the fold is `BankBook`'s own (`bank_book.h:252-258`), reached
|
||||
through the new public member, never re-implemented in `import_plan`. Sample display names
|
||||
are **not** suffixed, and `slot_map` positions are untouched.
|
||||
- **Always a new bank; never a merge (Ε-F2, ruled).** The import creates a bank — it never
|
||||
merges into an existing one, never lands into the pool, and offers no target picker. A
|
||||
pool export therefore lands as a **named** bank `"Pool 2"`, which is correct, not a glitch.
|
||||
This track ships **one** action, not two; merge-into-existing is out of scope for the
|
||||
phase, and move/copy already cover the after-the-fact case.
|
||||
- **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 back into the project that already contains B lands
|
||||
a **new** bank named `"B 2"`, with every id reminted, no entry lost, and B itself
|
||||
unmutated. Importing it a third time lands `"B 3"`.
|
||||
- The suffix probe is pinned by pure tests over a name set, covering at minimum: a free seed
|
||||
(no suffix applied), a case/whitespace-folded collision (`"drums"` blocks `"Drums"`), a gap
|
||||
(`"Drums"` + `"Drums 3"` present ⇒ `"Drums 2"`), a seed that already ends in a number
|
||||
(`"Drums 2"` colliding ⇒ `"Drums 2 2"`), an absent/blank recorded name (⇒ `Imported bank`),
|
||||
and a package whose source bank was the pool (⇒ `"Pool 2"`, a named bank).
|
||||
- A degraded ledger (`Unreadable` and `FutureVersion`, both asserted) refuses the import with
|
||||
**no picker shown, zero files written, zero index mutation**, and the message names the
|
||||
channel-correct ext-state namespace. An undecodable `rsusage_*` key with an otherwise
|
||||
`Loaded` ledger **does not** block — asserted, because the tempting reuse of
|
||||
`blockedByTracking` would silently make it.
|
||||
- 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.** **No [Daniel] questions — Ε-F2 and Ε-F3 are both RULED** (new bank
|
||||
always with an auto suffix; refuse on a degraded ledger), so this track is dispatchable as
|
||||
written and ships one action. **[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. Note the refusal path is
|
||||
already fixed at a console block by the Ε-F3 spec, so this call is about the *success*
|
||||
summary only.
|
||||
**Landed** — see `docs/COMPLETED.md` for the full narrative. New
|
||||
`core/package/import_plan` (pure: the id remap table, the parent remap, the per-entry
|
||||
land/skip-already-present/rename disposition, and the destination bank's display name
|
||||
after `BankBook`'s own uniqueness fold — reached through a new additive
|
||||
`BankBook::uniqueDisplayName` member, the only `core/model/` edit in the phase), and on
|
||||
the shell side a REAPER-free `import_landing` (decode, verify every payload's
|
||||
`hashBytes` digest against the manifest BEFORE the bank folder is created, then land
|
||||
through the rollback journal) plus a REAPER-facing `import_bank` (the only piece
|
||||
touching the extension's project state — the undo-batched persist and the generation
|
||||
bump), `shell/actions/package_import_action`, the panel's `.rsbank` drop route, one
|
||||
`main.cpp` row, one panel menu row, and a new `src/core/util/ascii_ws.h`. The
|
||||
tracking-ledger guard runs before the file picker opens (Ε-F3, ruled: refuse outright on
|
||||
`Unreadable`/`FutureVersion`, no confirm-and-proceed); the version gate runs before any
|
||||
byte is written; all four collision classes — sample id, file name, content hash, bank
|
||||
display name — are answered explicitly, with the display-name collision auto-suffixed
|
||||
and never prompted (Ε-F2, ruled: always a new bank, never a merge); birth records land
|
||||
via `recordCreated(sample, OriginKind::PackageImport)` in the same straight-line block
|
||||
as the index add; the index mutation is one Ctrl-Z, and the landed files' survival as
|
||||
orphans until the next prune is stated in the user-facing summary, not left implicit.
|
||||
**Beyond spec:** `import_plan`'s `spelledLikeABankFile` mints a fresh name even absent a
|
||||
collision, whenever the package's own entry name isn't spelled the way
|
||||
`deriveBankPaths` would spell it — counted separately from a genuine folder-name
|
||||
collision (`sanitizeRenameCount` vs `collisionRenameCount`) so a hostile or
|
||||
foreign-spelled entry name (e.g. an unexpected extension) always lands sanitized rather
|
||||
than verbatim.
|
||||
|
||||
---
|
||||
|
||||
@@ -2843,52 +2571,17 @@ 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.
|
||||
**Ε-W3-T1 has landed** — `package-compat-fixtures` — see `docs/COMPLETED.md` for the full
|
||||
narrative. 23 frozen `.rsbank` fixtures now live under `tests/fixtures/package_compat/`
|
||||
(three version fixtures, nine truncations, eleven hostile-name packages, ~15 KB total),
|
||||
decoded by a new `package_compat_tests` and driven through export → import → export by a
|
||||
new `package_round_trip_tests`, plus the DAW script `docs/verify-package-transfer.md` for
|
||||
the one claim no unit test can make. A repo-root `.gitattributes` (`*.rsbank binary`)
|
||||
keeps the frozen bytes frozen under `core.autocrlf`. No production module was touched.
|
||||
**Open question resolved:** the recommendation was followed — the corpus is one-sample
|
||||
packages with a 300-byte payload each. **Deviation from spec:** RSBK stores no layout
|
||||
section (it is derived from the manifest), so the planned "mid-layout" truncation lands
|
||||
at the payload boundary instead, where the manifest parses but the exact-size proof fails.
|
||||
|
||||
---
|
||||
|
||||
|
||||
+39
-1
@@ -1,6 +1,6 @@
|
||||
# DAW verification — post-1.0 work on `dev`
|
||||
|
||||
Checks for Θ, Ξ, and Ψ work that no unit test can close. Build **Release**, install into
|
||||
Checks for Θ, Ξ, Ψ, and Ε work that no unit test can close. Build **Release**, install into
|
||||
`UserPlugins/`, restart REAPER. Panel tail toggle = **None**, project rate 48000, unless a check says otherwise.
|
||||
|
||||
## Precision invariants
|
||||
@@ -59,6 +59,44 @@ Checks for Θ, Ξ, and Ψ work that no unit test can close. Build **Release**, i
|
||||
- [ ] Drag-out to an external app twenty-plus times in a row — audio arrives every time; this is a soak, a single pass is not a gate (`docs/COMPLETED.md:109`)
|
||||
- [ ] Drop a capture onto an FX container — the instrument loads with that capture (`docs/COMPLETED.md:110`)
|
||||
|
||||
## Bank packages
|
||||
|
||||
- [ ] **Run in full.** `docs/verify-package-transfer.md` — the whole cross-machine
|
||||
export/import round trip: writes-one-file, the transfer itself, re-importing the
|
||||
same file never overwrites, the round trip back to the source, the too-new /
|
||||
truncated / mid-payload-corruption refusals (each an exact string), the
|
||||
unsaved-project refusals, and drag-and-drop (`docs/COMPLETED.md:1068`, `PLAN.md:2578`)
|
||||
- [ ] Force a degraded tracking ledger and confirm the import refuses **before the
|
||||
file picker opens**: save a project with a bank, close REAPER, edit the saved
|
||||
`.rpp`'s `owned_files` ext-state value inside its `<REASAMPLER ...>` block — corrupt
|
||||
the JSON for the `Unreadable` case, or bump `"v":2` to `"v":3` for the
|
||||
`FutureVersion` case — reopen the project, then run *ReaSampler: import bank
|
||||
package (.rsbank)*. Read off: the console prints the ledger-refusal block and no
|
||||
file dialog ever appears (`src/core/tracking/origin_ledger.h:95,101`,
|
||||
`src/shell/actions/package_import_action.cpp:161-167`)
|
||||
- [ ] Export dialog: type a destination name with no extension, then again over a
|
||||
name that already carries a different one (e.g. `mybank.bak`) — read off whether
|
||||
`GetUserFileName` appended `.rsbank` itself or ReaSampler's own re-append produced
|
||||
the double-extension result (`mybank.bak.rsbank`) the code expects
|
||||
(`src/shell/package/CLAUDE.md:112-122`)
|
||||
- [ ] Both the export and the import file dialogs open in front of REAPER's main
|
||||
window, not behind it — `GetUserFileName` takes no owner window
|
||||
(`src/shell/package/CLAUDE.md:110-112`)
|
||||
- [ ] With a ReaSampler 9000 instance's editor open on the destination project
|
||||
(Browse view visible), import a `.rsbank` from the docked panel — the browser
|
||||
reflects the new bank without closing or reopening the editor (the bank-generation
|
||||
bump, `src/shell/persist/session.h:114-121`, polled by the instrument at
|
||||
`src/shell/instrument/processor_reload.cpp:444-475`)
|
||||
- [ ] Drag two or more `.rsbank` files onto the docked panel in one drop — each lands
|
||||
as its OWN new bank, never merged into one, and if the tracking ledger is degraded
|
||||
the refusal prints ONCE for the whole drop rather than once per file
|
||||
(`src/shell/panel/panel_window.cpp:63-94`)
|
||||
- [ ] Kill REAPER (or the process) partway through an import so a partial bank file
|
||||
is stranded under its real name in the bank folder, then re-run the same import
|
||||
into the same project — read off what happens. Whether the import verb should
|
||||
pre-clean that stale debris is an open question, not yet decided
|
||||
(`src/shell/package/CLAUDE.md:102-108`)
|
||||
|
||||
## The resample bake
|
||||
|
||||
- [ ] Bake a dialed sound — the banked file sounds like what the editor was playing (`docs/COMPLETED.md:737`)
|
||||
|
||||
@@ -381,7 +381,7 @@ trim, the seed is the literal `Imported bank`.
|
||||
|
||||
**The probe.** Let `seed` be that string and `fold(x)` be `BankBook`'s own uniqueness
|
||||
key — strip leading/trailing ASCII whitespace, lower-case ASCII letters
|
||||
(`bank_book.h:252-258`). Take the **first** name in this sequence whose fold is not
|
||||
(`bank_book.h:263-269`). Take the **first** name in this sequence whose fold is not
|
||||
already carried by a bank in the destination book:
|
||||
|
||||
seed, seed + " 2", seed + " 3", seed + " 4", …
|
||||
@@ -410,7 +410,7 @@ implementations diverge:**
|
||||
`B + 1` candidates is free by pigeonhole, so no cap is needed and none should be
|
||||
added.
|
||||
4. **The fold has exactly one home.** `import_plan` must **not** re-implement
|
||||
`nameKey` — `bank_book.h:252-258` says in as many words that a drifted second copy
|
||||
`nameKey` — `bank_book.h:263-269` says in as many words that a drifted second copy
|
||||
would let the uniqueness invariant be violated. The probe therefore runs behind
|
||||
`BankBook`'s own folding, which means Ε-W2-T2 adds **one additive public `const`
|
||||
member** to `BankBook` (recommended: `std::string uniqueDisplayName(const
|
||||
@@ -690,7 +690,7 @@ constructors.
|
||||
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
|
||||
(`src/shell/persist/session.h:121`, 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
|
||||
@@ -725,6 +725,30 @@ contemplates one untracked capture, an import strands hundreds).
|
||||
|
||||
---
|
||||
|
||||
## Implementation decisions — Ε-W2-T1
|
||||
|
||||
Not [Daniel]-class forks — both were `[propose at review]` calls in `docs/PLAN.md`'s
|
||||
Ε-W2-T1 track, answered at implementation review rather than by Daniel, and recorded
|
||||
here per this phase's own convention for keeping such answers where the design lives
|
||||
rather than only in the track's own now-stale open-questions line.
|
||||
|
||||
- **Affordance: both the bindable action and the panel row.** The action targets the
|
||||
**active** bank and is the only spelling that can reach the **pool** (the panel's
|
||||
`showTabMenu` returns early on `isPool()` — a named-bank-tab context menu has no tab
|
||||
to right-click for the pool), while the exported unit's own definition above includes
|
||||
the pool. The panel row is the direct gesture on a specific named bank. Neither
|
||||
subsumes the other.
|
||||
- **Default file name: the bank's display name**, sanitized through
|
||||
`capture_paths::sanitizeStem`, seeded into `<projectDir>/<stem>.rsbank`. A
|
||||
project-derived name was the rejected alternative: three banks exported from one
|
||||
project must produce three distinguishable files, and a project-derived name
|
||||
collides on the second export. Known wart, worth recording rather than hiding:
|
||||
`sanitizeStem` collapses an all-non-ASCII display name to the literal `capture`, so
|
||||
two such banks still collide — the existing rename verb is the recovery, same as the
|
||||
import-side auto-suffix collisions above.
|
||||
|
||||
---
|
||||
|
||||
## Non-goals and guardrails
|
||||
|
||||
- **No auto-insertion of imported audio into the arrange.** Same rule as capture.
|
||||
|
||||
@@ -0,0 +1,172 @@
|
||||
# DAW verification — bank-package transfer across machines
|
||||
|
||||
What a DAW pass must establish for `.rsbank` export and import, and the exact strings or
|
||||
counts to read off. The unit corpus (`tests/fixtures/package_compat/`) already proves the
|
||||
version ladder, the truncation verdicts and the hostile-name refusals against frozen
|
||||
bytes. **Nothing below is covered by it**: every cell here depends on a real REAPER
|
||||
session, a real file dialog, or a genuine second machine.
|
||||
|
||||
**Build to use.** Release, installed into `UserPlugins/`, REAPER restarted — extensions
|
||||
load at startup only. Note the version the *About*/version action reports; §5 needs it.
|
||||
|
||||
**Machines to use.** Two: **A** (the source) and **B** (the destination). B must be a
|
||||
different machine, or at minimum a different user account with its own REAPER resource
|
||||
path and its own projects folder — the point is that no absolute path from A can resolve
|
||||
on B. A USB stick, a network share, or a cloud folder are all acceptable transports.
|
||||
|
||||
**Projects to use.** On A: one **saved** project with a bank holding at least **three**
|
||||
samples, at least one of them audibly distinct from the others, and at least one whose
|
||||
display name carries a non-ASCII character (e.g. `Café hit`). On B: one **saved**,
|
||||
otherwise empty project.
|
||||
|
||||
---
|
||||
|
||||
## 1. Export writes one file and touches nothing else
|
||||
|
||||
On A, right-click the bank's header in the docked panel → **Export as package...** (or
|
||||
run *ReaSampler: export active bank as package*). Accept the suggested file name.
|
||||
|
||||
Read off:
|
||||
|
||||
- The console shows `ReaSampler export: wrote 3 entry/entries (N bytes) to <path>`, with
|
||||
the entry count matching the bank.
|
||||
- A single `.rsbank` file exists at that path. **No `.rsbanktmp` sibling remains** — a
|
||||
leftover temp file means the atomic rename did not complete.
|
||||
- The bank's card count, the bank folder's file count, and the project's dirty flag are
|
||||
all **unchanged**. An export writes no ext state and opens no undo point, so REAPER
|
||||
must not consider the project modified by it alone.
|
||||
- Nothing was added to the arrange view.
|
||||
|
||||
## 2. The transfer itself — the claim no unit test can make
|
||||
|
||||
Copy the `.rsbank` to B by whatever transport you chose. Do **not** copy the project, the
|
||||
bank folder, or anything else.
|
||||
|
||||
On B, open the empty saved project. Panel bank menu → **Import bank package...** (or run
|
||||
*ReaSampler: import bank package (.rsbank)*), and choose the transferred file.
|
||||
|
||||
Read off:
|
||||
|
||||
- A message box: `Imported 3 sample(s) into a new bank: "<bank name>".`
|
||||
- The console block repeats that line and ends with `One undo removes the imported bank
|
||||
and its entries. It does NOT delete the imported files ...`.
|
||||
- The panel shows a **new** bank with the same display name and the same number of cards,
|
||||
**in the same order** as on A.
|
||||
- B's bank folder holds three new files. The non-ASCII display name from A renders
|
||||
correctly on the card — a mangled name here means the UTF-8 path/name conversion broke
|
||||
in transit.
|
||||
- **Audition each card.** They must sound like their counterparts on A. This is the whole
|
||||
claim: the audio survived a machine boundary with no shared path.
|
||||
- Press **Ctrl-Z once**. The imported bank and its entries disappear in one step. The
|
||||
three files remain in B's bank folder (that is stated in the console block above, and is
|
||||
the designed behaviour — a prune reclaims them). Redo to continue.
|
||||
|
||||
## 3. Re-importing the same package never overwrites
|
||||
|
||||
Still on B, import the **same** file a second time.
|
||||
|
||||
Read off:
|
||||
|
||||
- A second new bank appears, named with a suffix (`<bank name> 2`), and the box's
|
||||
`(a bank named "<bank name>" already exists in this project)` clause appears in the
|
||||
console block.
|
||||
- B's bank folder now holds **six** files, not three. The console reports
|
||||
`3 file(s) landed under a freshly minted name (the package's own name was already taken
|
||||
in the bank folder). An existing bank file is never overwritten.`
|
||||
- The first imported bank's cards still audition correctly — nothing was replaced under it.
|
||||
|
||||
## 4. Round trip back to the source
|
||||
|
||||
On B, export the imported bank (§1) to a second `.rsbank`. Carry it back to A and import
|
||||
it into A's original project.
|
||||
|
||||
Read off:
|
||||
|
||||
- The import succeeds and lands as a new bank beside the original.
|
||||
- The original bank on A is untouched: same card count, same names, same audio.
|
||||
- Compare the two `.rsbank` files' **sizes**. They will usually differ — entry names,
|
||||
sample ids and the export timestamp are all legitimately re-minted across a trip. The
|
||||
payload bytes are what must survive, and that half is closed by
|
||||
`tests/test_package_round_trip.cpp` against frozen bytes; do **not** treat a size
|
||||
difference here as a defect.
|
||||
|
||||
## 5. The too-new refusal, with the message read verbatim
|
||||
|
||||
This is the direction a user hits when a collaborator is ahead of them, and the message is
|
||||
the only actionable output. Produce it by hand:
|
||||
|
||||
1. Copy the `.rsbank` from §1 to a scratch name.
|
||||
2. Open the copy in a hex editor. Bytes 0–3 are `RSBK`; bytes 4–7 are `formatVersion`
|
||||
little-endian; bytes **8–11** are `minReaderVersion` little-endian.
|
||||
3. Change byte **8** from `01` to `02`, and byte **4** from `01` to `02` (a writer cannot
|
||||
require a reader newer than the format it wrote — leaving `formatVersion` at 1 makes
|
||||
the file incoherent and it will be refused as malformed instead, which is a different
|
||||
cell). Save.
|
||||
4. Import the edited copy.
|
||||
|
||||
Read off — the message box, all four lines:
|
||||
|
||||
```
|
||||
Cannot import this bank package.
|
||||
It was written by ReaSampler <the version noted at the top> and needs package format 2 or newer.
|
||||
This build (<the same version>) reads package format 1.
|
||||
Nothing was imported. Install ReaSampler <the same version> or newer and try again.
|
||||
```
|
||||
|
||||
- The writer version named is the one **this** build stamped in §1 (the hex edit does not
|
||||
touch the semver), so the second and fourth lines will name your own version. That is
|
||||
expected — what is being verified is that all three facts are present and the box
|
||||
appears at all.
|
||||
- **No** new bank, **no** new files in the bank folder, **no** undo point.
|
||||
|
||||
## 6. The truncated-download refusal is a different message
|
||||
|
||||
Copy the §1 package again and delete the last few hundred bytes (any hex editor, or
|
||||
`head -c` / `fsutil` — the exact count does not matter as long as the file is shorter).
|
||||
Import it.
|
||||
|
||||
Read off:
|
||||
|
||||
- The message box reads exactly: `This file is not a readable bank package (corrupt or
|
||||
truncated). Nothing was imported.`
|
||||
- It is **not** the §5 message. Crossing these two is the failure this cell exists to
|
||||
catch — "install a newer build" does not fix a partial download.
|
||||
- No new bank, no new files.
|
||||
|
||||
## 7. Corruption in the middle is caught before anything lands
|
||||
|
||||
Copy the §1 package again and flip a single byte **well past the halfway point** (inside a
|
||||
payload, not the header). Import it.
|
||||
|
||||
Read off:
|
||||
|
||||
- The message box names the offending entry:
|
||||
`This bank package is damaged (entry "<name>" failed its integrity check). Nothing was
|
||||
imported.`
|
||||
- The bank folder gained **no** files at all — not even the entries before the damaged
|
||||
one. Verification runs to completion before the first write, so a damaged package costs
|
||||
no rollback.
|
||||
|
||||
## 8. The unsaved-project refusals
|
||||
|
||||
- On B, File → New Project (do not save). Try to import. Read off:
|
||||
`Save the project before importing a bank package -- an unsaved project has no bank
|
||||
folder to import into.` The file picker must **not** have opened first.
|
||||
- On A, in an unsaved project with no bank, try to export. Read off the console:
|
||||
`ReaSampler export: save the project first -- an unsaved project has no bank folder to
|
||||
read from.`
|
||||
|
||||
## 9. Drag-and-drop reaches the same verb
|
||||
|
||||
On B, drag a `.rsbank` from the file manager onto the docked ReaSampler panel.
|
||||
|
||||
Read off: the same import box as §2, and the same new bank. A `.rsbank` is a whole bank,
|
||||
not audio — it must never land as an item in the arrange view.
|
||||
|
||||
---
|
||||
|
||||
## Recording the result
|
||||
|
||||
For each section, record **pass**, **fail with the string actually seen**, or **not
|
||||
exercised**. §2 and §4 are the load-bearing ones: they are the only cells in this document
|
||||
that involve a real machine boundary, and no unit test can stand in for them.
|
||||
Reference in New Issue
Block a user