Take the read-back back out of the persist; a write verdict belongs only where evidence crosses the plugin boundary

Its false gated six undo points, so an unverified byte-equality assumption could have silently removed Ctrl-Z for a bank mutation that landed.
This commit is contained in:
2026-08-02 13:42:51 -04:00
parent 25390d5253
commit 1c8709e82d
12 changed files with 181 additions and 127 deletions
+20 -5
View File
@@ -305,6 +305,16 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) {
return out;
}
// The clause an ingest success line needs when the persist no-opped. saveToActiveProject
// returns false for exactly two reasons, no active project and an unsaved one, and in both
// the copied file is on disk with its entry in memory while NOTHING stored the index — so
// no message may report the import flatly.
std::string unsavedProjectNote(bool persisted) {
if (persisted) return {};
return " The bank index is IN MEMORY ONLY -- there is no saved project to store it in, "
"so save the project to keep it.";
}
// Imports the Media Explorer's last-played/selected file into the active bank, then
// adds a ReaSampler 9000 instrument to the FIRST SELECTED TRACK pre-loaded with that
// sound — no new track, no routing changes. No assignment_request write.
@@ -350,10 +360,11 @@ void doImportFromMediaExplorer() {
if (!target) {
// Bank import is kept (sound is in the bank browser); generation is bumped
// so any open VST3 browser instances refresh to show the new sound.
bool persisted = true; // true when nothing needed persisting (dedup)
if (r.added) {
Undo_BeginBlock2(nullptr);
g_session->bumpBankGeneration();
const bool persisted = g_session->saveToActiveProject();
persisted = g_session->saveToActiveProject();
if (persisted)
Undo_EndBlock2(nullptr, "ReaSampler: import Media Explorer file into bank",
UNDO_STATE_MISCCFG);
@@ -364,7 +375,8 @@ void doImportFromMediaExplorer() {
ShowConsoleMsg(("ReaSampler ingest: " + r.message +
" -- select a track first, then import into it "
"(sound is in the bank but no instrument was placed because "
"no track was selected).\n").c_str());
"no track was selected)." + unsavedProjectNote(persisted) +
"\n").c_str());
return;
}
@@ -392,10 +404,12 @@ void doImportFromMediaExplorer() {
bankPanelRefresh();
if (placed)
ShowConsoleMsg(("ReaSampler ingest: " + r.message +
" (loaded into a new instrument on the selected track).\n").c_str());
" (loaded into a new instrument on the selected track)." +
unsavedProjectNote(persisted) + "\n").c_str());
else
ShowConsoleMsg(("ReaSampler ingest: imported to the bank (" + r.message +
") but could not add the instrument to the selected track.\n").c_str());
") but could not add the instrument to the selected track." +
unsavedProjectNote(persisted) + "\n").c_str());
}
} // namespace
@@ -446,7 +460,8 @@ void ingestDroppedFiles(const std::vector<std::string>& absolutePaths) {
bankPanelRefresh();
const std::string msg =
"ReaSampler ingest: imported " + std::to_string(importedTotal) +
(importedTotal == 1 ? " file" : " files") + " into the bank.\n";
(importedTotal == 1 ? " file" : " files") + " into the bank." +
unsavedProjectNote(persisted) + "\n";
ShowConsoleMsg(msg.c_str());
} else if (importedTotal > 0) {
bankPanelRefresh();
+1 -1
View File
@@ -57,7 +57,7 @@ detail not covered there:
- `render_selection` (`shell/capture`) — the transient track selection a selected-tracks render (`&128`) requires, as a stack RAII guard: REAPER prints whatever tracks are selected, so `renderOffline` makes the request's own tracks BE the selection for the render's duration and restores the user's set on every exit path. Engaged ONLY for that source mode, which leaves a stated residual: a `&32` selected-items render still prints whatever ITEMS the user has selected. Live captures are unaffected (that selection is the source), but a recipe replay of a `SelectedItems` capture renders against whatever happens to be selected then — the recipe stores tracks and a range, never item GUIDs, so this guard cannot close it. Filed in `docs/TODO.md`.
- `render_isolation` (`shell/capture`) — the transient upstream silencing a ranged ITEM render needs, as a stack RAII guard alongside the two above: the selected-tracks source prints everything flowing INTO the track, so each direct folder child's `B_MAINSEND` and each of the track's receives' `B_MUTE` are cut for the render and restored on every exit path. Direct children only — a grandchild reaches the track through the child that owns it. The child-set walk is pure (`core/capture/track_topology`).
- `capture_orchestrator` (`shell/capture`) — single-capture orchestration + the realtime/insert action bodies (Q-W3 hoist, T4-02): `renderOffline` (one offline render under the scope's FX-bypass guard), `captureAndIndexOne` (render + provenance stamp + bank add + tracking-ledger record, unpersisted), `RunCapture`/`RunCaptureItemAssign`, `RunCaptureRealtimeTrack`/`RunCancelRealtime` (the realtime action bodies — the in-flight state lives in `realtime_lifecycle`), and `RunInsertSelected` (the ONE deliberate exception to capture-never-places).
- `bake_land` (`shell/capture`) — the EXTENSION's half of the resample chain, the SCAN PASS: scans every open project tab for pending `rsbake_*` requests, lands the ones belonging to the project this session has loaded (via `bake_landing`, below), and refuses the rest with `WrongProject` — one undo point for the batch, each answered over its own key inside the invoking instance's synchronous action call. It owns every ext-state read and write in the chain. The per-key verdict itself is NOT this TU's: it is `core/wire`'s pure `classifyBakeScan`, so this shell only enumerates, reads, and applies — counting every verdict into a `wire::BakeScanTally` as it goes, printing `wire::describeBakeKey` for EVERY enumerated key (the only thing that names which key is whose) plus `wire::describeBakeScan` whenever any key went unanswered or any answer's write was not confirmed, in one `ShowConsoleMsg`. It PROVES every write — answer or stale-clear — by reading the key back (`wire::extStateWriteLanded`, whose home is `core/wire/ext_state_read.h`); an answer that did not land is the one no-answer the tally alone cannot show. That proof is three-valued (`wire::BakeWriteProof`): a read-back that overflowed, or a throw AFTER the `SetProjExtState` call, reports Unknown; a throw BEFORE it reports Rejected, because the write is then known not to have been made. Each key is materialized before any answer is written, so no `SetProjExtState` in this action mutates a set the enumerator is still walking. Answers are held UNENCODED until after the pass's single persist, so a landing the project would not take is answered as a failure rather than as an `Ok` no reload would honour — `wire::bakeLandingAfterPersist` is the ONE route to a `Banked` landing, and no path here (dedup included) may assign that word itself. The undo block is stack RAII (`UndoBlock`). Both loops are guarded: a throw in the scan still writes the answers already prepared, and a throw in the write-back loop still prints the lines already accumulated — no path through this action can end in a silent console. It RENDERS NOTHING — the instrument already did, through its own engine in its own process, which is what makes the baked audio the sound the user approved and what keeps the voice engine out of the extension's link graph.
- `bake_land` (`shell/capture`) — the EXTENSION's half of the resample chain, the SCAN PASS: scans every open project tab for pending `rsbake_*` requests, lands the ones belonging to the project this session has loaded (via `bake_landing`, below), and refuses the rest with `WrongProject` — one undo point for the batch, each answered over its own key inside the invoking instance's synchronous action call. It owns every ext-state read and write in the chain. The per-key verdict itself is NOT this TU's: it is `core/wire`'s pure `classifyBakeScan`, so this shell only enumerates, reads, and applies — counting every verdict into a `wire::BakeScanTally` as it goes, printing `wire::describeBakeKey` for EVERY enumerated key (the only thing that names which key is whose) plus `wire::describeBakeScan` whenever any key went unanswered or any answer's write was not confirmed, in one `ShowConsoleMsg`. It PROVES every write — answer or stale-clear — by reading the key back (`wire::extStateWriteLanded`, whose home is `core/wire/ext_state_read.h`); an answer that did not land is the one no-answer the tally alone cannot show. That proof is three-valued (`wire::BakeWriteProof`): a read-back that overflowed, or a throw AFTER the `SetProjExtState` call, reports Unknown; a throw BEFORE it reports Rejected, because the write is then known not to have been made. Each key is materialized before any answer is written, so no `SetProjExtState` in this action mutates a set the enumerator is still walking. Answers are held UNENCODED until after the pass's single persist, so a landing whose pass never got its persist through is answered as a failure rather than as an `Ok` no reload would honour — `wire::bakeLandingAfterPersist` is the ONE route to a `Banked` landing, and no path here (dedup included) may assign that word itself. The undo block is stack RAII (`UndoBlock`). Both loops are guarded: a throw in the scan still writes the answers already prepared, and a throw in the write-back loop still prints the lines already accumulated — no path through this action can end in a silent console. It RENDERS NOTHING — the instrument already did, through its own engine in its own process, which is what makes the baked audio the sound the user approved and what keeps the voice engine out of the extension's link graph.
- `bake_landing` (`shell/capture`) — landing ONE bake request, split off `bake_land` on the one-request / whole-pass seam; touches no REAPER API at all. Non-mutating `prepareLanding` and mutating `commitLanding` sit under separate catches in `attemptLanding` — a throw before anything was written is a clean refusal, a throw after it is reported as possibly partial. Replace-vs-add comes from `tracking::resampleLanding`; a replace keeps the entry's id and slot and never deletes the superseded file. Hash-dedup applies on the add path only, before the disk write, matching `updateSampleInPlace`'s "an in-place refresh is not an insert" — and a dedup hit still rides the pass's persist, because the entry it points at may be one the same pass just added. A refused index withdraws the bytes this call had just written — the self-cleanup carve-out from prune's deletion authority, stated in `prune_fs.cpp`'s header. It never persists: the pass does that once for its whole batch, which is why no landing may report itself as banked.
- `capture_batch` (`shell/capture`) — the batch-capture family + re-capture-from-source (Q-W3 hoist, T4-02): `RunBatchCaptureItems` (one sample per selected item), `RunBatchCaptureRazor` (one sample per razor area), `RunRecaptureFromSource` (regenerate a provenanced sample from its recorded source's current state, bank-only). Every unit routes through `capture_orchestrator` so every precision invariant holds; persist is batched to one ext-state write per action.
- `realtime_lifecycle` (`shell/capture`) — the in-flight realtime-capture state machine + globals (Q-W3 hoist): the action starts it, `OnTimer` drives it per tick via `DriveRealtimeCapture` (a single-pointer-test idle fast path — load-bearing hot-path guardrail), `CommitRealtimeResult` lands a finished capture in the bank, `AbortRealtimeCaptureForUnload` tears down cleanly on extension unload.
+5 -6
View File
@@ -139,17 +139,16 @@ wire::BakeWriteProof writeBackOne(ScannedKey& entry, bool persisted) {
std::string value; // empty = clear the key
if (entry.write == ScannedKey::Write::Answer) {
// Where a landing earns the word: the pass's persist runs after the whole scan,
// so until here nothing had observed whether the project took it. Without it
// the entry lives in memory alone, which is a failed bake to anyone who
// reloads — so it is answered as one rather than as an Ok.
// so until here no bank write had been issued for it at all. Without one the
// entry lives in memory alone, which is a failed bake to anyone who reloads —
// so it is answered as one rather than as an Ok.
const wire::BakeLanding after =
wire::bakeLandingAfterPersist(entry.report.landing, persisted);
if (after == wire::BakeLanding::Unpersisted) {
entry.outcome = refuseBake(
BakeStatus::Failed,
"the bake reached the bank in memory, but this pass's persist did not "
"confirm its bank write, so this answer cannot promise a reload will "
"find it",
"report success, so this answer cannot promise a reload will find it",
entry.outcome.generation);
// `detail` left empty on purpose: describeLanding's Unpersisted clause is
// that sentence's one home on the console side, and the message above is
@@ -196,7 +195,7 @@ void RunResampleBake(ReaSamplerSession& session) {
std::vector<ScannedKey> scanned;
wire::BakeScanTally tally; // every verdict below is counted, skips included
std::string aborted; // set only when the scan itself threw
bool persisted = false; // the pass's ONE persist confirmed its bank write
bool persisted = false; // the pass's ONE persist ran and reported success
bool bookChanged = false; // some landing added or refreshed an entry
// A local, not a tally field: an entry counted here can still be re-answered as a
// failure by the write-back loop below, so this is a persist GATE and never a count
+1 -1
View File
@@ -53,7 +53,7 @@ REAPER/filesystem-facing half only, and it gathers rather than decides.
## Modules
- `shell/persist` (`session` / `ext_state_io` / `prune_fs`) — the persist seam, split by responsibility (Q-W5; the former `persist.cpp` god-TU and its `persist.h` compatibility umbrella are both retired — callers include `shell/persist/session.h` / `ext_state_io.h` directly). `session` owns the `ReaSamplerSession` lifecycle: the poll identity-transition detection (load / Save-As / forked sibling / recycled pointer) and the `projectconfig`-driven deferred undo/redo reload. `ext_state_io` owns project ext state (`SetProjExtState`/`GetProjExtState`, namespace `"reasampler"`) ↔ `BankBook` JSON, `ViewModeModel` JSON, `TailSetting` JSON, the tracking ledger JSON, the writing-version stamp, GUID minting, and bank-folder relocation. `saveToActiveProject` PROVES its `banks` write by reading that key back (`wire::extStateWriteLanded`) and returns that verdict — the sibling keys are written but not verified, so its `true` means "the bank state is in the project", never "everything persisted". `session` additionally owns `recordCreated`**the one writer of a birth record**, called at the same point the `Sample` is added, deriving lineage from that `Sample`'s own provenance. `prune_fs` hosts the prune dry-run / full-set orphan queries (gathering `referencedPaths()` plus `tracking::pruneProtection`'s two inputs for the `prune_reconcile` pure core) — and, beside them, `tiedUsageFor`, the resample's replace-vs-add input, deliberately co-located so "both answers come out of one `TrackingState`" is structural rather than a rule two files must remember. It is also **the single file-deletion authority over user files in the bank folder** (`deleteOrphanFile` via `SHFileOperationW`); nothing else in the system deletes bank-folder bytes. Dry-run / orphan-set / reclaim each independently abort (delete nothing) when the authority reports a block.
- `shell/persist` (`session` / `ext_state_io` / `prune_fs`) — the persist seam, split by responsibility (Q-W5; the former `persist.cpp` god-TU and its `persist.h` compatibility umbrella are both retired — callers include `shell/persist/session.h` / `ext_state_io.h` directly). `session` owns the `ReaSamplerSession` lifecycle: the poll identity-transition detection (load / Save-As / forked sibling / recycled pointer) and the `projectconfig`-driven deferred undo/redo reload. `ext_state_io` owns project ext state (`SetProjExtState`/`GetProjExtState`, namespace `"reasampler"`) ↔ `BankBook` JSON, `ViewModeModel` JSON, `TailSetting` JSON, the tracking ledger JSON, the writing-version stamp, GUID minting, and bank-folder relocation. `saveToActiveProject` returns whether the writes were ISSUED — false means no active/saved project and NOTHING was written, which is the only reading its callers' discard-the-undo-point branch is safe under; it must never grow an observational third failure mode (the contract lives at its declaration in `session.h`). `session` additionally owns `recordCreated`**the one writer of a birth record**, called at the same point the `Sample` is added, deriving lineage from that `Sample`'s own provenance. `prune_fs` hosts the prune dry-run / full-set orphan queries (gathering `referencedPaths()` plus `tracking::pruneProtection`'s two inputs for the `prune_reconcile` pure core) — and, beside them, `tiedUsageFor`, the resample's replace-vs-add input, deliberately co-located so "both answers come out of one `TrackingState`" is structural rather than a rule two files must remember. It is also **the single file-deletion authority over user files in the bank folder** (`deleteOrphanFile` via `SHFileOperationW`); nothing else in the system deletes bank-folder bytes. Dry-run / orphan-set / reclaim each independently abort (delete nothing) when the authority reports a block.
- `usage_scan` — extension-side prune-scan shell: enumerates every `rsusage_*` ext-state key, decodes each `sample_usage` wire record, enumerates every ReaSampler 9000 FX instance across all tracks + master / normal + record chains / containers (recursive) / take FX, and returns the pure `sample_usage::foldUsageRecords` result verbatim. One of the two inputs `tracking::pruneProtection` reads; it decides nothing itself. Read-only: writes no ext-state.
- `persist_internal.h` — internal-only shared helpers for the persist TU family (`session` / `ext_state_io` / `prune_fs`); included only by those three TUs, never a public seam (mirror of the panel's `panel_state.h` / the editor's `editor_internal.h` precedent). Holds the former anonymous-namespace helpers more than one split TU needs (active-project + `.rpp` path lookup, project-dir derivation, growing `GetProjExtState` read, project-GUID minting, bank-folder relocation) — all definitions live in `ext_state_io.cpp`. REAPER-free header: the project handle crosses this seam as the same opaque `void*` the public `session` header already uses.
+4 -17
View File
@@ -183,23 +183,10 @@ bool ReaSamplerSession::saveToActiveProject() {
MarkProjectDirty(static_cast<ReaProject*>(proj));
// Prove the ONE key a caller's answer hinges on. Each SetProjExtState above returns the
// size of the whole extname's state, which the six writes here keep non-zero between
// them, so no single one of those returns can speak for `banks`
// (wire::extStateWriteLanded owns the reasoning). The sibling keys stay unobserved and
// no caller claims otherwise; `banks` is the one whose absence would make a landed
// capture vanish on reload.
const wire::GrowingExtStateRead back = wire::readProjExtStateGrowing(
[&](char* buf, int cap) {
return GetProjExtState(static_cast<ReaProject*>(proj), projExtNamespace(),
kProjExtBanksKey, buf, cap);
});
if (back.status == wire::GrowingExtStateRead::Status::Overflow)
return false; // could not check -> do not claim; never folded in as an absence
return wire::extStateWriteLanded(
banksJson, back.status == wire::GrowingExtStateRead::Status::Complete
? std::optional<std::string>(back.value)
: std::nullopt);
// The writes were ISSUED into a saved active project — all this call can observe, and
// deliberately all it claims. Do not "prove" them with a read-back; session.h states
// what a false has to keep meaning to its callers, and why.
return true;
}
bool ReaSamplerSession::writeAssignmentRequest(const std::string& wire) {
+10 -10
View File
@@ -108,16 +108,14 @@ public:
void bumpBankGeneration() { ++bankGeneration_; }
// Serializes book/view/tail to ext state, clears the retired legacy
// `bank_index` key. No-ops with no active/saved project.
// `bank_index` key. No-ops with no active/saved project. Returns true iff
// a persist happened, so a caller can skip an undo block when nothing was written.
//
// Returns true iff the `banks` key READ BACK as exactly what this call wrote
// the only per-key observation available under a shared extname
// (wire::extStateWriteLanded owns why SetProjExtState's own return cannot
// answer it). A false therefore covers four things without distinguishing
// them: no active project, an unsaved project, a rejected write, and a
// read-back that could not complete. The sibling keys (view/tail/ledger/
// version/generation) are written but NOT verified, so no caller may read
// this as "everything persisted" — only as "the bank state is in the project".
// "Happened" is the writes being ISSUED, and a false means exactly two things
// no active project, or an unsaved one — with NOTHING written under either. SIX
// callers turn a false straight into a discarded undo point, so this must never
// grow a third, OBSERVATIONAL failure mode: a false negative would silently
// remove the Ctrl-Z for a bank mutation that landed.
bool saveToActiveProject();
// Report-only prune dry-run: feeds the pure core with (present, referenced,
@@ -151,7 +149,9 @@ public:
// Write the ingest assignment request (`assign_request` key): "the active
// sampler instance should now play THIS sample." `wire` is pre-encoded
// (assignment_request.h); a sibling one-shot write, not part of
// saveToActiveProject's blob. Returns true iff written.
// saveToActiveProject's blob. Same two-condition return as that call: false
// means no active/saved project and nothing was written, true means the write
// was ISSUED — never that the project took it.
bool writeAssignmentRequest(const std::string& wire);
// Detects a project load or Save-As and reacts. Driven by REAPER's