feat(persist): persist BankBook under banks key, retire legacy bank_index, route capture to active bank (B2)

src/bank_book.cpp

@@ -668,4 +668,24 @@ std::optional<BankBook> BankBook::deserialize(const std::string& json) {
     return book;
 }
 
+BankBook BankBook::loadFromPersisted(const std::string& banksJson,
+                                     const std::string& legacyJson) {
+    // Precedence 1: the authoritative `banks` blob. A present-but-malformed blob is
+    // an error, not an absence — degrade to an empty book rather than falling through
+    // to a stale legacy key (which would resurrect superseded single-bank state).
+    if (!banksJson.empty()) {
+        auto book = deserialize(banksJson);
+        return book ? std::move(*book) : BankBook{};
+    }
+    // Precedence 2: no `banks` yet, but a legacy `bank_index` — one-way pool migration
+    // (deserialize's parse-time legacy path promotes it into the pool). A malformed
+    // legacy blob likewise degrades to empty.
+    if (!legacyJson.empty()) {
+        auto book = deserialize(legacyJson);
+        return book ? std::move(*book) : BankBook{};
+    }
+    // Precedence 3: a brand-new / never-captured project — a fresh empty book.
+    return BankBook{};
+}
+
 } // namespace reasampler

src/bank_book.h

@@ -190,6 +190,22 @@ public:
     // shape going forward; the legacy key is retired by the B2 shell).
     static std::optional<BankBook> deserialize(const std::string& json);
 
+    // Resolve a BankBook from the two persisted ext-state values a project may carry:
+    // the authoritative `banks` blob and the retired-but-possibly-present legacy
+    // `bank_index` blob. The persist shell (B2) hands both raw strings straight here so
+    // the load-source decision stays REAPER-free and unit-tested. Precedence:
+    //   1. non-empty `banksJson` present  -> deserialize it (authoritative). If it is
+    //      MALFORMED, do NOT silently fall back to the legacy blob — a corrupt `banks`
+    //      blob is an error, not an absence; return an empty book so a stale legacy key
+    //      can never resurrect a superseded single-bank state over a broken book.
+    //   2. else non-empty `legacyJson`    -> deserialize it (one-way pool migration).
+    //   3. else (both absent/empty)        -> a fresh empty book (pool only).
+    // Never returns nullopt: an unloadable input degrades to the empty book (matching
+    // the shell's existing "malformed -> ignore, start empty" behaviour), so the caller
+    // has one branchless install path.
+    static BankBook loadFromPersisted(const std::string& banksJson,
+                                      const std::string& legacyJson);
+
 private:
     std::vector<Bank> banks_;         // ordinal order; banks_[0] is always the pool
     std::string       activeBankId_;  // always names a live bank; defaults to pool

src/main.cpp

@@ -111,9 +111,10 @@ static int g_cmdCancelRealtime = 0;
 
 // The persistence session (M4): owns the in-memory BankIndex and bridges it to
 // project ext state. A timer tick drives g_session.poll() to detect project
-// load / Save-As; capture adds Samples to g_session.bank(); after a capture we
-// serialize the bank back into the active project's ext state so it travels with
-// the .rpp. Replaces the M3 session-only g_bank.
+// load / Save-As; capture adds Samples to g_session.bank() — which (B2) resolves to
+// the ACTIVE bank's index inside the session's BankBook; after a capture we serialize
+// the book back into the active project's ext state (the `banks` key) so it travels
+// with the .rpp. Replaces the M3 session-only g_bank.
 static reasampler::ReaSamplerSession g_session;
 
 // --- M8 in-flight realtime capture (async, timer-driven) --------------------
@@ -133,8 +134,9 @@ static reasampler::RealtimeCaptureHandle    g_rtCapture;
 static ReaProject* g_rtCaptureProject = nullptr;
 
 // Commit a finished realtime capture (a Done tick/abort with an Ok result): add the
-// Sample to the bank, persist + MarkProjectDirty, log. Shared by the tick-completion
-// path and the abort paths. On a non-Ok result, logs the failure only.
+// Sample to the ACTIVE bank (g_session.bank() resolves to book.activeIndex() — B2),
+// persist + MarkProjectDirty, log. Shared by the tick-completion path and the abort
+// paths. On a non-Ok result, logs the failure only.
 static void CommitRealtimeResult(const reasampler::CaptureResult& res)
 {
     if (res.status != reasampler::CaptureStatus::Ok)
@@ -547,10 +549,12 @@ static void RunCapture(const reasampler::CaptureActionDef& def)
         return;
     }
 
+    // Add to the ACTIVE bank: g_session.bank() resolves to book.activeIndex() (B2).
     reasampler::AddResult added = g_session.bank().add(res.sample);
-    // Persist the updated bank into the active project's ext state so the capture
-    // survives Save / close+reopen (M4) and travels with the .rpp. saveToActiveProject
-    // also calls MarkProjectDirty. Non-destructive: writes only our own ext-state key.
+    // Persist the updated book into the active project's ext state (the `banks` key)
+    // so the capture survives Save / close+reopen (M4) and travels with the .rpp.
+    // saveToActiveProject also clears the retired legacy key and calls MarkProjectDirty.
+    // Non-destructive: writes only our own ext-state keys.
     g_session.saveToActiveProject();
 
     std::string log = "ReaSampler: " + res.message + "\n";

src/persist.cpp

@@ -5,11 +5,15 @@
 // WITHOUT REAPERAPI_IMPLEMENT — main.cpp is the one TU that defines the API
 // pointers; here they are extern (CLAUDE.md §contract).
 //
-// Storage: SetProjExtState / GetProjExtState, namespace "reasampler", key
-// "bank_index". Ext state is stored INSIDE the .rpp, so the index travels with
-// the project automatically (CONTEXT.md §Persistence & paths). The only thing
-// that does NOT travel for free is the physical bank folder; on Save-As to a new
-// directory we relocate it so the index's relative paths still resolve.
+// Storage: SetProjExtState / GetProjExtState, namespace "reasampler". Phase B: the
+// whole BankBook (pool as bank-zero + named banks) is written under key "banks"
+// (authoritative); the legacy single-bank key "bank_index" is RETIRED — cleared on
+// save (SetProjExtState with "" deletes it) and read only once, to migrate a pre-
+// multi-bank project's index into the pool. Ext state is stored INSIDE the .rpp, so
+// the banks travel with the project automatically (CONTEXT.md §Persistence & paths).
+// The only thing that does NOT travel for free is the physical bank folder; on
+// Save-As to a new directory we relocate it so the indices' relative paths still
+// resolve.
 //
 // PROJECT-LOAD / SAVE-AS DETECTION (chosen mechanism):
 //   Driven by REAPER's "timer" register (main.cpp). Each poll() reads the active
@@ -174,12 +178,22 @@ void ReaSamplerSession::saveToActiveProject() {
     if (!proj) return;                 // no active project — nothing to persist
     if (rppPath.empty()) return;       // unsaved project — no .rpp to store into
 
-    const std::string json = bank_.serialize();
+    // Phase B: the whole book (pool as bank-zero + named banks) is authoritative and
+    // rides in the `banks` key.
+    const std::string banksJson = book_.serialize();
     SetProjExtState(static_cast<ReaProject*>(proj), kProjExtNamespace,
-                    kProjExtIndexKey, json.c_str());
+                    kProjExtBanksKey, banksJson.c_str());
 
-    // Additive: the Design-View model rides alongside the bank in its own key.
-    // Independent write — does not disturb the bank_index above.
+    // Retire the legacy single-bank `bank_index` key: SetProjExtState with an empty
+    // value DELETES the key (SDK header ~6288: val NULL or "" deletes the data). This
+    // realizes retirement concretely — after any save, a formerly-legacy project
+    // carries `banks` and NO `bank_index`, and going forward the legacy key is never
+    // written. Cheap and idempotent when the key is already absent.
+    SetProjExtState(static_cast<ReaProject*>(proj), kProjExtNamespace,
+                    kProjExtIndexKey, "");
+
+    // Additive: the Design-View model rides alongside the banks in its own key.
+    // Independent write — does not disturb the `banks` blob above.
     const std::string viewJson = view_.serialize();
     SetProjExtState(static_cast<ReaProject*>(proj), kProjExtNamespace,
                     kProjExtViewKey, viewJson.c_str());
@@ -226,32 +240,46 @@ void ReaSamplerSession::loadFromProject(void* proj, const std::string& projectDi
     view_ = loadViewModel(static_cast<ReaProject*>(proj));
 
     if (!proj) {
-        bank_ = BankIndex{};
+        book_ = BankBook{};
         return;
     }
-    const std::string json =
-        getProjExtStateString(static_cast<ReaProject*>(proj), kProjExtNamespace,
-                              kProjExtIndexKey);
-    if (json.empty()) {
-        // No stored index (new or never-captured project) — start empty.
-        bank_ = BankIndex{};
-        return;
-    }
-    std::optional<BankIndex> loaded = BankIndex::deserialize(json);
-    if (!loaded) {
-        ShowConsoleMsg("ReaSampler: stored bank index is malformed — ignoring.\n");
-        bank_ = BankIndex{};
-        return;
-    }
-    bank_ = std::move(*loaded);
 
-    // Project-relative resolution is a READ-time concern: the index stores only
-    // relative paths (invariant), and consumers (M5 panel, M6 insert) resolve
-    // each entry against the CURRENT project dir via resolveBankFile(projectDir,
-    // relativePath). We do NOT rewrite the stored paths to absolute here — that
-    // would break the relative-only invariant and the travel-with-.rpp property.
-    // projectDir is threaded through for those consumers; nothing to do at load
-    // time beyond replacing the in-memory bank.
+    // Read both possible sources: the authoritative `banks` blob and the retired-but-
+    // possibly-still-present legacy `bank_index`. The precedence + migration decision
+    // (`banks` wins; else the legacy index migrates into the pool; else an empty book)
+    // is pure logic; it is inlined here rather than via BankBook::loadFromPersisted only
+    // so a malformed `banks` blob can be warned on the console (single parse) — a corrupt
+    // blob must read as "ignored", not silent loss, mirroring the prior malformed-index
+    // warning. A malformed `banks` degrades to an empty book and does NOT fall back to
+    // the stale legacy key (which would resurrect superseded single-bank state).
+    const std::string banksJson =
+        getProjExtStateString(static_cast<ReaProject*>(proj), kProjExtNamespace,
+                              kProjExtBanksKey);
+    if (!banksJson.empty()) {
+        std::optional<BankBook> loaded = BankBook::deserialize(banksJson);
+        if (!loaded) {
+            ShowConsoleMsg("ReaSampler: stored banks are malformed — ignoring.\n");
+            book_ = BankBook{};
+        } else {
+            book_ = std::move(*loaded);
+        }
+    } else {
+        // No `banks` yet — fall back to the legacy `bank_index`, migrated into the pool
+        // by BankBook's parse-time promotion. loadFromPersisted covers the legacy-or-
+        // empty tail; passing "" for banksJson takes exactly that branch.
+        const std::string legacyJson =
+            getProjExtStateString(static_cast<ReaProject*>(proj), kProjExtNamespace,
+                                  kProjExtIndexKey);
+        book_ = BankBook::loadFromPersisted(std::string{}, legacyJson);
+    }
+
+    // Project-relative resolution is a READ-time concern: every BankIndex in the book
+    // stores only relative paths (invariant, enforced per-bank at add()), and consumers
+    // (M5 panel, M6 insert) resolve each entry against the CURRENT project dir via
+    // resolveBankFile(projectDir, relativePath). We do NOT rewrite stored paths to
+    // absolute here — that would break the relative-only invariant and travel-with-.rpp.
+    // projectDir is threaded through for those consumers; nothing to do at load time
+    // beyond replacing the in-memory book.
     (void)projectDir;
 }
 

src/persist.h

@@ -19,6 +19,7 @@
 
 #include <string>
 
+#include "bank_book.h"
 #include "bank_model.h"
 #include "view_mode_model.h"
 
@@ -28,10 +29,21 @@ namespace reasampler {
 // shipped: changing it orphans every already-saved project's index.
 inline constexpr const char* kProjExtNamespace = "reasampler";
 
-// The ext-state key the index JSON is stored under (one key holds the whole
-// serialized BankIndex). FOREVER-STABLE for the same reason.
+// The RETIRED legacy ext-state key: pre-multi-bank projects stored the whole
+// serialized BankIndex here (single bank). Phase B2 no longer WRITES it — on save
+// the key is cleared (SetProjExtState with "" deletes it) and the book is written
+// under kProjExtBanksKey instead. It is still READ once, on load of a legacy
+// project, to migrate its single index into the pool (BankBook's parse-time
+// promotion). FOREVER-STABLE as a read key for that migration path.
 inline constexpr const char* kProjExtIndexKey = "bank_index";
 
+// The multi-bank ext-state key (Phase B): one key holds the whole serialized
+// BankBook — the pool folded in as bank-zero plus every named bank, each with its
+// own BankIndex, ordinals, and the active-bank id. AUTHORITATIVE going forward;
+// supersedes kProjExtIndexKey. FOREVER-STABLE once shipped: changing it orphans
+// every already-saved project's banks.
+inline constexpr const char* kProjExtBanksKey = "banks";
+
 // The ext-state key the Design-View ViewModeModel JSON is stored under (one key
 // holds the whole serialized model: modes + membership + show-both + snapshots +
 // active mode). Distinct from kProjExtIndexKey — one namespace, two keys.
@@ -45,8 +57,9 @@ inline constexpr const char* kProjExtViewKey = "view_state";
 // it strands the identity of every already-saved project. See persist.cpp.
 inline constexpr const char* kProjExtGuidKey = "project_guid";
 
-// Owns the session's BankIndex and drives persistence against the active REAPER
-// project. One instance lives for the extension's lifetime (main.cpp). It tracks
+// Owns the session's BankBook (Phase B: pool + named banks) and drives persistence
+// against the active REAPER project. One instance lives for the extension's
+// lifetime (main.cpp). It tracks
 // the project identity it last saw so the timer tick can detect a project load
 // (a different project became active) and a Save-As (SAME project, path changed):
 //
@@ -62,16 +75,28 @@ inline constexpr const char* kProjExtGuidKey = "project_guid";
 // W12 defect that stopped the bank reloading); the pointer catches forks (Save-As
 // copies our GUID onto a distinct object — the W10 defect that clobbered a bank).
 //
-// The bank itself is exposed for the capture/action layer to mutate; persist
+// The book itself is exposed for the capture/action layer to mutate; persist
 // only reads it on save and replaces it on load.
 class ReaSamplerSession {
 public:
     ReaSamplerSession() = default;
 
-    // The in-memory bank. The action/capture layer adds captures here; persist
-    // serializes it on save and replaces it on project load.
-    BankIndex& bank() { return bank_; }
-    const BankIndex& bank() const { return bank_; }
+    // The multi-bank book (Phase B): the pool + named banks, each wrapping a
+    // BankIndex, plus the active-bank id. The action layer (B3) creates / renames /
+    // reorders / deletes banks and moves samples here; the panel (B4) reads it;
+    // persist serializes it under the `banks` key on save and replaces it on load.
+    BankBook& book() { return book_; }
+    const BankBook& book() const { return book_; }
+
+    // The capture add-target: the ACTIVE bank's BankIndex (defaults to the pool).
+    // The capture path adds a captured Sample through this seam, so a capture lands
+    // in whichever bank is active — the single behavioural change B2 wires in over
+    // M7/M8 (the capture backends are untouched; only the target index moved). The
+    // panel/insert readers that displayed the single index continue to read it here
+    // unchanged; today it resolves to the pool (default active), matching prior
+    // single-bank behaviour, until B3/B4 let the user switch the active bank.
+    BankIndex& bank() { return book_.activeIndex(); }
+    const BankIndex& bank() const { return book_.activeIndex(); }
 
     // The in-memory Design-View model. The view/action layer mutates it (tag,
     // toggle, snapshot); persist serializes it on save and replaces it on project
@@ -80,8 +105,9 @@ public:
     ViewModeModel& view() { return view_; }
     const ViewModeModel& view() const { return view_; }
 
-    // Serialize the current bank to the active project's ext state (namespace
-    // "reasampler"). Non-destructive beyond writing our own ext-state key. Safe
+    // Serialize the current book (under the `banks` key) and view model to the active
+    // project's ext state (namespace "reasampler"), and clear the retired legacy
+    // `bank_index` key. Non-destructive beyond writing our own ext-state keys. Safe
     // to call when there is no active/saved project (it no-ops).
     void saveToActiveProject();
 
@@ -103,7 +129,7 @@ public:
     bool consumeLoadSignal();
 
 private:
-    BankIndex bank_;
+    BankBook book_;
 
     // The Design-View model. Default-constructed = Arrange + Design seeded, active
     // = Arrange; loadFromProject leaves this default when a project has no stored
@@ -124,9 +150,10 @@ private:
     bool        primed_ = false;          // false until the first poll() observes state
     bool        loadPending_ = false;     // raised by loadFromProject; drained by consumeLoadSignal
 
-    // Load the index from the given project's ext state and resolve bank paths
-    // against projectDir. Replaces the in-memory bank. projectDir empty -> clears
-    // the bank (unsaved project has no resolvable bank).
+    // Load the book from the given project's ext state (the `banks` key, else the
+    // legacy `bank_index` key migrated into the pool) and resolve bank paths against
+    // projectDir at read time. Replaces the in-memory book. projectDir empty -> the
+    // book is reset to empty (unsaved project has no resolvable banks).
     void loadFromProject(void* proj, const std::string& projectDir);
 };
 

tests/test_bank_book.cpp

@@ -341,6 +341,70 @@ static void testMalformedJson() {
     CHECK(!BankBook::deserialize(dup).has_value());
 }
 
+// --- B2: persist-load source-precedence decision (pure) --------------------
+// loadFromPersisted picks the load source the persist shell will feed it from the
+// two ext-state values a project may carry: the authoritative `banks` blob and the
+// retired legacy `bank_index` blob. Precedence: banks > legacy > empty; a malformed
+// banks blob degrades to empty WITHOUT falling back to the stale legacy key.
+
+static void testLoadPrefersBanksBlob() {
+    // A full book + a stale legacy index both present: `banks` wins, legacy ignored.
+    BankBook src;
+    CHECK(src.createBank("drums", "Drums"));
+    CHECK(src.setActiveBank("drums"));
+    CHECK(src.bank("drums")->index.add(sampleWith("new1")) == AddResult::Added);
+    const std::string banksJson = src.serialize();
+
+    BankIndex stale;
+    CHECK(stale.add(sampleWith("stale-old")) == AddResult::Added);
+    const std::string legacyJson = stale.serialize();
+
+    BankBook loaded = BankBook::loadFromPersisted(banksJson, legacyJson);
+    // The book equals the source book — the legacy key had NO effect.
+    CHECK(loaded == src);
+    CHECK(loaded.activeBankId() == "drums");
+    CHECK(loaded.bank("drums") != nullptr);
+    CHECK(loaded.bank("drums")->index.query("id-new1") != nullptr);
+    // The stale legacy sample must NOT have leaked into the pool.
+    CHECK(loaded.pool().index.query("id-stale-old") == nullptr);
+}
+
+static void testLoadMigratesLegacyWhenNoBanks() {
+    // No `banks` key, a legacy `bank_index` present: migrate into the pool, zero named.
+    BankIndex legacy;
+    CHECK(legacy.add(sampleWith("l1")) == AddResult::Added);
+    CHECK(legacy.add(sampleWith("l2")) == AddResult::Added);
+    const std::string legacyJson = legacy.serialize();
+
+    BankBook loaded = BankBook::loadFromPersisted(std::string{}, legacyJson);
+    CHECK(loaded.size() == 1);                                   // pool only
+    CHECK(loaded.pool().id == kPoolBankId);
+    CHECK(loaded.activeBankId() == std::string(kPoolBankId));
+    CHECK(loaded.pool().index.size() == 2);
+    CHECK(loaded.pool().index == legacy);                       // lossless
+}
+
+static void testLoadEmptyWhenNeither() {
+    // Both absent: a fresh empty book (pool only, empty index, active = pool).
+    BankBook loaded = BankBook::loadFromPersisted(std::string{}, std::string{});
+    CHECK(loaded == BankBook{});
+    CHECK(loaded.size() == 1);
+    CHECK(loaded.pool().index.empty());
+    CHECK(loaded.activeBankId() == std::string(kPoolBankId));
+}
+
+static void testLoadMalformedBanksDegradesWithoutLegacyFallback() {
+    // A present-but-malformed `banks` blob must degrade to an empty book and must NOT
+    // resurrect the stale legacy key (that would revive superseded single-bank state).
+    BankIndex stale;
+    CHECK(stale.add(sampleWith("stale")) == AddResult::Added);
+    const std::string legacyJson = stale.serialize();
+
+    BankBook loaded = BankBook::loadFromPersisted("{\"banks\":[", legacyJson);
+    CHECK(loaded == BankBook{});                                // empty, NOT the legacy
+    CHECK(loaded.pool().index.query("id-stale") == nullptr);   // legacy did not leak
+}
+
 static void testActiveBankResolveAfterCorruptPersistedId() {
     // A book blob whose activeBank names no bank resolves to the pool (defensive).
     const char* json =
@@ -366,6 +430,10 @@ int main() {
     testJsonEmptyBookRoundTrip();
     testLegacyMigration();
     testMalformedJson();
+    testLoadPrefersBanksBlob();
+    testLoadMigratesLegacyWhenNoBanks();
+    testLoadEmptyWhenNeither();
+    testLoadMalformedBanksDegradesWithoutLegacyFallback();
     testActiveBankResolveAfterCorruptPersistedId();
 
     if (g_fail == 0) std::printf("All tests passed.\n");