diff --git a/docs/TODO.md b/docs/TODO.md index 300e686..27a93a6 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -767,3 +767,83 @@ above. function under a `_tests` target with no REAPER, no VST3 SDK, and no filesystem includes; `shell/capture/render_bounds_gate` shrinks to the file-move and the two callers' plumbing. + +## The capture path ignores `saveToActiveProject`'s return at four sites + +**Context.** `saveToActiveProject()` returns false for exactly two reasons — no active +project, or an unsaved one — and in both cases NOTHING was written. Four capture sites +discard that return outright: `capture_orchestrator.cpp:343`, `capture_batch.cpp:266` and +`:333`, and `realtime_lifecycle.cpp:39`. + +**The wart.** A capture on an unsaved project renders the file into the bank folder, adds +the `Sample` to the in-memory book, records a birth record in memory — and loses all three +on reload. The bytes stay on disk with no index entry and no persisted ledger record, so +they are a foreign file prune will never reclaim (an unrecorded file is untouchable by +design — `core/tracking/CLAUDE.md`). Nothing is printed. The bank-op family already reads +this return and discards its undo point on a false; the capture family does not read it at +all. + +**Why filed, not fixed.** Pre-existing, and the right answer is a product decision this +dispatch had no mandate for: refuse the capture up front, keep it and warn, or prompt for +a Save-As (the bank ops chose "quiet persist by design, deliberately no Save-As prompt" — +whether capture should follow is a separate call). + +**Done looks like.** A capture attempted with no saved project either does not write bytes +at all, or writes them and says so in a sentence naming what will not survive a reload — +and the choice between those two is recorded rather than implicit. + +## `panel_input`'s wheel handler persists the whole book per wheel message + +**Context.** `panel_input.cpp:450` — `handleWheel` calls `markTailDirty()` on every wheel +message that actually moves `manualMs`, while the pointer is over the footer in Manual +mode. (It coalesces sub-notch deltas within ONE message and no-ops at a bound, so the +count is wheel messages that changed the value, not raw notches.) + +**The wart.** `markTailDirty` is `saveToActiveProject()` — a full `BankBook` serialize plus +six ext-state value writes on the UI thread — for a setting that is one number. A flick +over the footer is a dozen of them in a few hundred milliseconds. Disproportionate rather +than incorrect: no guardrail is violated (this is nowhere near the two named hot paths), +and the writes are idempotent. + +**Intended fix.** Coalesce: mark dirty and let one timer tick flush, the same shape the +panel already uses elsewhere for repaint batching. + +**Done looks like.** A continuous wheel gesture over the footer produces one persist, and +the value that lands is the gesture's final one. + +## `RunCaptureItemAssign`'s undo point does not follow the pattern its comment claims + +**Context.** `capture_orchestrator.cpp:364-365` states that the action follows the bank-op +family's discard-on-unsaved pattern. + +**The wart.** It does not: `:382-383` records the undo point unconditionally whenever +`sampleId` is non-empty, and never consults the persist's return at all. So on an unsaved +project it records an undo point for ext-state that was never written — the empty +no-effect entry `persistBankOp`'s guardrail exists to avoid. The comment describes the +intended behavior, not the code. + +**Why filed, not fixed.** It is one instance of the capture-family gap filed above, and +fixing it alone would leave the other four sites divergent. Fix them together, or explain +in one place why capture differs from bank ops. + +**Done looks like.** The comment and the code agree, and the whole capture family answers +the unsaved-project case one way. + +## `core/tracking/CLAUDE.md`'s untracked-file enumeration says "reaches the `.rpp`" too loosely + +**Context.** `src/core/tracking/CLAUDE.md:24-31` enumerates how a created file can stay +untracked, and describes the ledger as reaching the `.rpp` at the following +`saveToActiveProject()`. + +**The wart.** `saveToActiveProject()` writes REAPER's IN-MEMORY project state and marks the +project dirty; REAPER writes the `.rpp` on the project's own save, which may be much later +or never. The sentence was already loose before this branch and is not made wrong by it — +but it is the same over-claim ("a write reached the file on disk") the bake's reporting +pass spent several rounds removing from its own sentences, so it should read the same way. + +**Why filed, not fixed.** Editing another layer's own CLAUDE.md from a persist-and-report +dispatch is exactly the boundary crossing the per-directory docs exist to prevent. It is a +doc-keeper edit. + +**Done looks like.** The enumeration distinguishes "in the project's state" from "on disk +in the `.rpp`", and does not gain a second home for the distinction. diff --git a/src/core/wire/CLAUDE.md b/src/core/wire/CLAUDE.md index f700b31..e12f482 100644 --- a/src/core/wire/CLAUDE.md +++ b/src/core/wire/CLAUDE.md @@ -77,10 +77,10 @@ This directory owns two cross-artifact contracts specifically: ## Modules -- `wire` (`core/wire`) — the ONE length-prefixed ext-state wire codec (Q-W1): `putField`/`parseUnsignedDecimal` + the bounds-checked `Cursor` (`field`/`fieldInt`/`fieldInt64`/`fieldSizeT`/`fieldDouble`), replacing four near-identical copies (`provenance` / `assignment_request` / `sample_usage` / `bank_sync`). `core/wire/bytes.h` is the sibling little-endian byte codec (`putLE`, `ByteReader`, `doubleToBits`/`bitsToDouble`) that `component_state_io` is the biggest consumer of. `core/wire/ext_state_read.h` owns the `GetProjExtState` grow-loop retry policy (Absent/Complete/Overflow) shared by `persist`, `usage_scan`, and `reaper_bridge`, and its peer `extStateWriteLanded` — the ONLY verdict on whether a `SetProjExtState` write took, because that API's return is the size of the WHOLE extname's state and cannot speak for one key. Three shells bind it: the bake landing's answer/clear write-back, `saveToActiveProject`'s `banks` write, and the instrument bridge's two prefix-guarded writes. Its `nullopt` means ABSENT specifically, so every caller must route an Overflow read to its own "could not check" answer rather than folding it in. `core/wire/reasampler_uid.h` (the FOREVER-FROZEN VST3 class-UID macros) also lives in this directory. +- `wire` (`core/wire`) — the ONE length-prefixed ext-state wire codec (Q-W1): `putField`/`parseUnsignedDecimal` + the bounds-checked `Cursor` (`field`/`fieldInt`/`fieldInt64`/`fieldSizeT`/`fieldDouble`), replacing four near-identical copies (`provenance` / `assignment_request` / `sample_usage` / `bank_sync`). `core/wire/bytes.h` is the sibling little-endian byte codec (`putLE`, `ByteReader`, `doubleToBits`/`bitsToDouble`) that `component_state_io` is the biggest consumer of. `core/wire/ext_state_read.h` owns the `GetProjExtState` grow-loop retry policy (Absent/Complete/Overflow) shared by `persist`, `usage_scan`, and `reaper_bridge`, and its peer `extStateWriteLanded` — the ONLY verdict on whether a `SetProjExtState` write took, because that API's return is the size of the WHOLE extname's state and cannot speak for one key. Two shells bind it, BOTH on the instrument's per-instance keys: the bake landing's answer/clear write-back and the instrument bridge's prefix-guarded writes. Deliberately NOT the persist, whose return is control flow over undo points — this verdict is a proof value reported to the user, and an observational false there would discard the Ctrl-Z for a bank mutation that landed. Its `nullopt` means ABSENT specifically, so every caller must route an Overflow read to its own "could not check" answer rather than folding it in. `core/wire/reasampler_uid.h` (the FOREVER-FROZEN VST3 class-UID macros) also lives in this directory. - `reasampler_uid.h` — SDK-free header owning the FOREVER-FROZEN VST3 class-UID integer macros (stable + beta pairs, `REASAMPLER_PROC_UID_*` / `REASAMPLER_PROC_UID_BETA_*`) and the `REASAMPLER_ACTIVE_UID_*` channel-selector macros. Split out of `reasampler_vst.h` so the pure extension side (`instrument_drop`) can derive the `.vstpreset` class-ID hex string without pulling in the VST3 SDK. Both `reasampler_vst.h` (runtime `FUID`) and `instrument_drop` (preset hex string) source from this single header — the binary identity and the preset-file identity cannot diverge. - `assignment_request` — pure ingest-assign wire: typed request record carrying the drop payload from the `ingest` shell through to the VST3 bridge. -- `bake_wire` — the resample bake's request/outcome pair on ONE per-instance key (`rsbake_`): the instrument writes a `BakeRequest`, invokes the extension's action synchronously, and reads the extension's `BakeOutcome` back over the same key inside that one call. Not a handshake — a call and a return, and it must not grow a claim protocol. Also the ONE home of the bake action's command-id suffix and of the leading underscore `NamedCommandLookup` needs but `rec->Register("command_id", …)` does not, so both artifacts name one action. `BakeStatus` values are WIRE INTEGERS: never renumber, only append, and an unrecognized value decodes as `Failed` rather than as the numeric default `Ok`. It owns BOTH ends' reading of that key, since the key's contents are the only evidence either side gets: `classifyBakeAnswer` (instrument side — six kinds, of which `Unanswered`, the request still sitting there untouched, separates "nothing wrote an outcome over our key" from a refusal — it does NOT identify a landing that never ran, since a skipped key and a rejected answer-write look identical from here) and `classifyBakeScan` + `kMaxRequestAgeSeconds` (extension side — the per-key Land / RefuseWrongProject / ClearStale / IgnoreUnreadable / IgnoreNotARequest verdict over every open tab, stated without a REAPER type so the multi-tab matrix is unit-provable). `BakeScanTally` + `describeBakeScan` + `describeBakeKey` are that same reading counted and spoken — the rationale lives at the types. **The report's absence is NOT evidence the landing never ran**, and no sentence either artifact prints may say it is: `answered` is pass-wide and counts a QUEUED write, so a pass can answer some other key while skipping ours, or have our own answer's `SetProjExtState` rejected. The summary is therefore silent only when the pass answered somebody, left no key unanswered, and every answer was READ BACK from its own key; `describeBakeKey` prints one line per enumerated key regardless, which is the only thing that names WHICH key — the counts cannot. The read-back verdict behind `BakeWriteProof` is `ext_state_read.h`'s `extStateWriteLanded`, above. `bakeLandingAfterPersist` is the ONE route to a `Banked` landing: every `Land` verdict is assigned `Unpersisted` and passed through it, so no shell path — a dedup hit least of all, since its target may be an entry the same pass just added — can claim the word without the pass's persist having CONFIRMED its bank write. What `Banked` claims is exactly that `persisted` input; the enum comment is that claim's one home. +- `bake_wire` — the resample bake's request/outcome pair on ONE per-instance key (`rsbake_`): the instrument writes a `BakeRequest`, invokes the extension's action synchronously, and reads the extension's `BakeOutcome` back over the same key inside that one call. Not a handshake — a call and a return, and it must not grow a claim protocol. Also the ONE home of the bake action's command-id suffix and of the leading underscore `NamedCommandLookup` needs but `rec->Register("command_id", …)` does not, so both artifacts name one action. `BakeStatus` values are WIRE INTEGERS: never renumber, only append, and an unrecognized value decodes as `Failed` rather than as the numeric default `Ok`. It owns BOTH ends' reading of that key, since the key's contents are the only evidence either side gets: `classifyBakeAnswer` (instrument side — six kinds, of which `Unanswered`, the request still sitting there untouched, separates "nothing wrote an outcome over our key" from a refusal — it does NOT identify a landing that never ran, since a skipped key and a rejected answer-write look identical from here) and `classifyBakeScan` + `kMaxRequestAgeSeconds` (extension side — the per-key Land / RefuseWrongProject / ClearStale / IgnoreUnreadable / IgnoreNotARequest verdict over every open tab, stated without a REAPER type so the multi-tab matrix is unit-provable). `BakeScanTally` + `describeBakeScan` + `describeBakeKey` are that same reading counted and spoken — the rationale lives at the types. **The report's absence is NOT evidence the landing never ran**, and no sentence either artifact prints may say it is: `answered` is pass-wide and counts a QUEUED write, so a pass can answer some other key while skipping ours, or have our own answer's `SetProjExtState` rejected. The summary is therefore silent only when the pass answered somebody, left no key unanswered, and every answer was READ BACK from its own key; `describeBakeKey` prints one line per enumerated key regardless, which is the only thing that names WHICH key — the counts cannot. The read-back verdict behind `BakeWriteProof` is `ext_state_read.h`'s `extStateWriteLanded`, above. `bakeLandingAfterPersist` is the ONE route to a `Banked` landing: every `Land` verdict is assigned `Unpersisted` and passed through it, so no shell path — a dedup hit least of all, since its target may be an entry the same pass just added — can claim the word without the pass's persist having reported success. What `Banked` claims is exactly that `persisted` input — a saved project was active and the bank write was issued — and the enum comment is that claim's one home. - `instrument_drop` — pure FX-drop payload builder: constructs a Steinberg-format `.vstpreset` image (channel-active class ID + the instrument's own component state, capture pre-selected) the shell applies via `TrackFX_SetPreset`; owns `classifyReaperSurface`, the prefix classifier mapping a `GetThingFromPoint` (info token, track-present) pair onto `core/ui/drag_out`'s `ReaperSurface`. Classifier ordering is load-bearing: the embed strip is matched before the `tcp`/`mcp` panel family, which now claims the WHOLE track panel rather than just its FX sub-elements. All-or-nothing contract — caller rolls back via `TrackFX_Delete` on any failure. - `sample_usage` — instance-usage wire: `UsageRecord`, `planUsagePublish` (fresh/heal/clean-replace/union/remint publish plan), `foldUsageRecords`/`usageHeldPaths` (liveness fold — protect-all when records exist but no instance is live; abort→protect-all on unreadable record; `counted` carries key-attributed live records), `identityMatches` (ReaSampler 9000 FX identity). REAPER-free, unit-tested. The mirror of `assignment_request` on the instrument→extension direction: the wire format and the two safety-critical decisions (what to write on publish, which records count at prune time) are pure so they are provable without a DAW. It lives here because it is a *wire format* with an instrument-side writer; the fold's output is consumed by `core/tracking`'s authority, which owns every consumer-facing decision built on it. diff --git a/src/core/wire/bake_wire.cpp b/src/core/wire/bake_wire.cpp index 821f722..d930ed0 100644 --- a/src/core/wire/bake_wire.cpp +++ b/src/core/wire/bake_wire.cpp @@ -246,18 +246,17 @@ namespace { std::string describeLanding(BakeLanding landing) { switch (landing) { case BakeLanding::Banked: - // Bounded by what `persisted` actually observed (see BakeLanding): the stored - // bank state read back as this pass wrote it. The trailing clause is an + // Bounded by what `persisted` actually observed (see BakeLanding): a saved + // project was active and the write was issued. The trailing clause is an // instruction, not a claim about the .rpp's current contents. - return "landed into the bank, and the project's stored bank state read back as " - "exactly what this pass wrote -- save the project to keep it"; + return "landed into the bank, and this pass issued its bank write into the " + "saved project -- save the project to keep it"; case BakeLanding::Unpersisted: - // Says nothing about what the project holds: the persist can fail before - // writing anything, throw part-way through, or write and fail to read back — - // and a dedup hit's target may have been in the project since long before this - // pass. + // Says nothing about what the project holds: the persist may never have run at + // all, and a dedup hit's target may have been in the project since long before + // this pass. return "landed into the bank IN MEMORY ONLY -- this pass's persist did not " - "confirm its bank write, so nothing here can say the project carries it"; + "report success, so nothing here can say the project carries it"; case BakeLanding::Partial: return "the landing failed after it had begun writing -- it may have left a " "file in the bank folder and an entry in memory"; diff --git a/src/core/wire/bake_wire.h b/src/core/wire/bake_wire.h index eecfe7a..8e072b0 100644 --- a/src/core/wire/bake_wire.h +++ b/src/core/wire/bake_wire.h @@ -196,8 +196,8 @@ std::string describeBakeScan(const BakeScanTally& tally); enum class BakeLanding { Refused, // nothing was written and the book is untouched Partial, // it threw AFTER it had begun writing — a file and/or an entry may exist - Unpersisted, // it reached the in-memory book; the pass's persist did not confirm - Banked, // in the book, and the pass's persist confirmed its bank write + Unpersisted, // it reached the in-memory book; the pass's persist did not report success + Banked, // in the book, and the pass issued its bank write into a saved project }; // The ONE route to `Banked`: every Land verdict's landing is assigned `Unpersisted` and @@ -205,11 +205,16 @@ enum class BakeLanding { // directly on the grounds that it changed nothing — was wrong precisely when the entry it // deduped against was one the SAME pass had just added and then failed to persist. // -// `persisted` is the shell's OBSERVATION that the project's stored bank state now reads -// back as what the pass wrote (ReaSamplerSession::saveToActiveProject, which proves that -// one key via extStateWriteLanded) — not that a save was merely attempted. Every sentence -// derived from `Banked` is bounded by that; it says nothing about the project's OTHER keys -// and nothing about the .rpp on disk, which REAPER writes on the project's own save. +// `persisted` is ReaSamplerSession::saveToActiveProject's return: a saved project was +// active and the bank write was ISSUED into it. That is the whole claim — not that REAPER +// took the value, not that any sibling key was written, and not that the .rpp on disk holds +// it, which REAPER writes on the project's own save. +// +// The `Unpersisted` limb survives that narrowing: under a Land verdict the persist's own +// two refusals are already excluded, so what reaches it is a persist that did not RUN — +// bake_land assigns the landing inside a guarded scan and sets its `persisted` local only +// in the block after it, so a throw between the two leaves a landed entry with the flag +// still false. BakeLanding bakeLandingAfterPersist(BakeLanding landing, bool persisted); // Whether the write a verdict required actually took. Three states, not two: the read-back diff --git a/src/core/wire/ext_state_read.h b/src/core/wire/ext_state_read.h index 03036fb..0113876 100644 --- a/src/core/wire/ext_state_read.h +++ b/src/core/wire/ext_state_read.h @@ -1,8 +1,8 @@ #pragma once -// ext_state_read — the GetProjExtState grow-loop retry policy AND its peer, the -// read-back verdict that says whether a write took, shared by the extension's -// persist/usage-scan/landing shells and the instrument's bridge so neither rule -// can drift between them. +// ext_state_read — the GetProjExtState grow-loop retry policy (shared by the +// extension's persist/usage-scan/landing shells and the instrument's bridge) AND +// its peer, the read-back verdict that says whether a write took — that one bound +// to the instrument's per-instance keys only, for the reason stated at it below. // // GetProjExtState writes into a caller-supplied buffer with no query-the-size // call, so a large value must be read by growing a buffer until it fits @@ -77,12 +77,25 @@ GrowingExtStateRead readProjExtStateGrowing(ReadFn&& read) { // per-key observation available under a shared extname. An empty `written` is a CLEAR, // which lands as an absent-or-empty key rather than as those bytes. // +// SCOPE — the per-instance keys the two artifacts exchange across the plugin/extension +// boundary. The one that earns it is the bake's `rsbake_`: the instrument publishes a +// request, REAPER dispatches the action, the extension answers over the same key, and an +// instance finding nothing there genuinely cannot tell "never written" from "written and +// did not stick" by any other means. (The `rsusage_` publish rides along because it shares +// the bridge's one prefix-guarded writer; its verdict is currently unread.) +// +// Do NOT bind it to a write both made and read by one call in one process, and above all +// not to one whose verdict becomes CONTROL FLOW: this is a PROOF VALUE reported to the +// user, never a decision about what to do next. `saveToActiveProject` is the standing +// counter-example — its return IS control flow, over undo points — so do not reintroduce +// a read-back there. +// // `[verify — DAW]` byte equality assumes REAPER stores and returns an ext-state value // verbatim. Two known ways that could be false: a value REAPER normalises on the round // trip, and an embedded NUL (SetProjExtState/GetProjExtState are C-string transports, so a -// payload containing one is truncated on write). Both fail in the safe direction — a write -// REAPER did take reads back as unconfirmed, never the reverse — so neither can turn an -// unconfirmed write into a claimed success. +// payload containing one is truncated on write). Either would report a write REAPER did +// take as unconfirmed — tolerable for a proof value, which is the only thing this may +// answer, and the reason the scope above is a hard limit rather than a preference. inline bool extStateWriteLanded(const std::string& written, const std::optional& readBack) { if (written.empty()) return !readBack || readBack->empty(); diff --git a/src/shell/actions/ingest.cpp b/src/shell/actions/ingest.cpp index e60f15d..65ce842 100644 --- a/src/shell/actions/ingest.cpp +++ b/src/shell/actions/ingest.cpp @@ -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& 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(); diff --git a/src/shell/capture/CLAUDE.md b/src/shell/capture/CLAUDE.md index 330cf8a..ea057b2 100644 --- a/src/shell/capture/CLAUDE.md +++ b/src/shell/capture/CLAUDE.md @@ -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. diff --git a/src/shell/capture/bake_land.cpp b/src/shell/capture/bake_land.cpp index d560760..6704755 100644 --- a/src/shell/capture/bake_land.cpp +++ b/src/shell/capture/bake_land.cpp @@ -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 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 diff --git a/src/shell/persist/CLAUDE.md b/src/shell/persist/CLAUDE.md index e310077..9447f06 100644 --- a/src/shell/persist/CLAUDE.md +++ b/src/shell/persist/CLAUDE.md @@ -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. diff --git a/src/shell/persist/ext_state_io.cpp b/src/shell/persist/ext_state_io.cpp index d4e38ab..59502d5 100644 --- a/src/shell/persist/ext_state_io.cpp +++ b/src/shell/persist/ext_state_io.cpp @@ -183,23 +183,10 @@ bool ReaSamplerSession::saveToActiveProject() { MarkProjectDirty(static_cast(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(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(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) { diff --git a/src/shell/persist/session.h b/src/shell/persist/session.h index 14d2043..ffbaad0 100644 --- a/src/shell/persist/session.h +++ b/src/shell/persist/session.h @@ -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 diff --git a/tests/test_bake_wire.cpp b/tests/test_bake_wire.cpp index 74a1cbb..3b71220 100644 --- a/tests/test_bake_wire.cpp +++ b/tests/test_bake_wire.cpp @@ -9,8 +9,8 @@ // degrading to Failed rather than to Ok; the action lookup name's leading underscore and // its channel fork; the two key classifiers each end reads the shared key through; the // write-back verdict, driven by a modelled key store that accepts or drops the write; and -// the persist/upgrade state machine that is the ONLY route to a Banked landing, including -// the reachability of its Unpersisted limb at both persist outcomes. +// the persist/upgrade state machine that is the ONLY route to a Banked landing, at both +// persist outcomes. #include "../src/core/wire/bake_wire.h" @@ -33,8 +33,8 @@ static int g_fail = 0; // here and used by both blocks below that need it, so the test suite does not become a // third place the sentence lives. static const std::string kUnpersistedAnswer = - "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"; + "the bake reached the bank in memory, but this pass's persist did not report success, " + "so this answer cannot promise a reload will find it"; int main() { // --- The exact bytes on the wire ------------------------------------------------- @@ -554,8 +554,10 @@ int main() { // rather than a keyed verdict and an unkeyed reason the reader has to pair up. CHECK(ok.find("(added as a distinct capture)") != std::string::npos); // The sentence names the OBSERVATION behind the flag (see BakeLanding) and stops - // there — it may not claim the .rpp on disk already holds the entry. - CHECK(ok.find("read back as exactly what this pass wrote") != std::string::npos); + // there — the write was ISSUED into a saved project. It may not claim REAPER took + // the value, nor that the .rpp on disk already holds the entry. + CHECK(ok.find("issued its bank write into the saved project") != std::string::npos); + CHECK(ok.find("read back") == std::string::npos); CHECK(ok.find(".rpp") == std::string::npos); CHECK(ok.find("carries it") == std::string::npos); @@ -585,11 +587,11 @@ int main() { unpersisted.landing = BakeLanding::Unpersisted; const std::string memoryOnly = describeBakeKey(key, unpersisted); CHECK(memoryOnly.find("IN MEMORY ONLY") != std::string::npos); - CHECK(memoryOnly.find("persist did not confirm its bank write") != std::string::npos); + CHECK(memoryOnly.find("persist did not report success") != std::string::npos); CHECK(memoryOnly != ok); - // It may NOT claim what the project's saved state holds: the persist can fail - // before writing anything or throw part-way, and a dedup hit's target may have - // been in the project since long before this pass. + // It may NOT claim what the project's saved state holds: the persist may never + // have run at all, and a dedup hit's target may have been in the project since + // long before this pass. CHECK(memoryOnly.find("does not carry it") == std::string::npos); BakeKeyOutcome partial = landed; @@ -686,6 +688,10 @@ int main() { // to be answered Banked directly, on the grounds that it changed nothing — false // exactly when the entry it deduped against was one the SAME pass had just added and // then failed to persist, which answers Ok for an entry the project does not carry. + // + // Both limbs are live in the shell, not just in this table: it assigns the landing + // inside a guarded scan and sets its `persisted` local only in the block after it, so a + // throw between the two reaches Unpersisted with a real landing behind it. { // A dedup hit and a fresh add are INDISTINGUISHABLE here, by construction: the shell // assigns Unpersisted to both, so both need the same observation to be promoted. @@ -741,8 +747,8 @@ int main() { // Judging this by SetProjExtState's return made `writeFailed` unproducible on the // landing path and its sentence dead code (extStateWriteLanded owns why). The verdict is // the read-back, which a store that drops the write does produce. The store is modelled - // here; the three shells bind these same two calls to SetProjExtState and the - // grow-loop read. + // here; the two shells that bind the verdict make these same two calls, to + // SetProjExtState and the grow-loop read. { struct FakeKeyStore { std::map values; @@ -837,56 +843,6 @@ int main() { CHECK(!extStateWriteLanded("", std::optional("leftover"))); } - // --- The Unpersisted limb is REACHABLE, through the same predicate ------------------ - // `persisted` used to be structurally true wherever a landing could observe it: the - // session reported it from SetProjExtState's unread return, and by the time a landing - // exists the only two conditions that return could reflect (no active project, an - // unsaved one) are already excluded by the Land verdict. So the Unpersisted limb, the - // wire answer for it and its console clause were all dead. The session now proves the - // `banks` key the same way the landing proves its own — modelled below at both - // outcomes, so neither branch is a constant. - { - const std::string banksJson = R"({"banks":[{"id":"pool","samples":[]}]})"; - - // The write REJECTED: the project still holds whatever it held before, which is a - // book without the entry this pass just landed in memory. - const std::optional stale{R"({"banks":[]})"}; - const bool persistedAfterDrop = extStateWriteLanded(banksJson, stale); - CHECK(!persistedAfterDrop); - - BakeKeyOutcome entry; - entry.verdict = BakeScanVerdict::Land; - entry.landing = - bakeLandingAfterPersist(BakeLanding::Unpersisted, persistedAfterDrop); - entry.proof = BakeWriteProof::Confirmed; // the ANSWER wrote fine; the persist did not - CHECK(entry.landing == BakeLanding::Unpersisted); - const std::string line = describeBakeKey("rsbake_0123abcd", entry); - CHECK(line.find("IN MEMORY ONLY") != std::string::npos); - CHECK(line.find("The answer was written back") != std::string::npos); - - // ...and that landing is answered as a FAILURE on the wire, so no instance adopts - // an entry a reload would not find. - BakeOutcome answered; - answered.status = BakeStatus::Failed; - answered.message = kUnpersistedAnswer; - answered.generation = 1893456000; - const auto back = decodeBakeOutcome(encodeBakeOutcome(answered)); - CHECK(back.has_value() && back->status == BakeStatus::Failed); - CHECK(back.has_value() && back->message == kUnpersistedAnswer); - - // The write ACCEPTED, same predicate, same inputs but for the read-back: the limb - // above is a real branch, not a constant. - const bool persistedAfterTake = - extStateWriteLanded(banksJson, std::optional(banksJson)); - CHECK(persistedAfterTake); - CHECK(bakeLandingAfterPersist(BakeLanding::Unpersisted, persistedAfterTake) == - BakeLanding::Banked); - // An unreadable read-back must NOT be folded in as an absence-and-therefore-a-clear: - // the session routes Overflow to false before it ever reaches this predicate, and a - // non-empty write against an absent key is false here regardless. - CHECK(!extStateWriteLanded(banksJson, std::nullopt)); - } - // --- Every outcome bake_land actually emits survives the key round trip ------------- // The landing writes these and the instrument reads them back; a field the encoder and // the decoder disagreed about would strand exactly the bake that produced it.