From 1800bd64c381b4077e0d8a275ea0e44a45b43827 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 12:00:34 -0400 Subject: [PATCH] Prove the bake's answer writes by reading the key back SetProjExtState's return covers the whole extname, so it never saw one key. The persist verdict now reaches the report, and a throw mid-write no longer claims the landing left nothing behind. --- docs/TODO.md | 21 +- src/core/wire/CLAUDE.md | 2 +- src/core/wire/bake_wire.cpp | 61 +++- src/core/wire/bake_wire.h | 45 ++- src/shell/capture/CLAUDE.md | 2 +- src/shell/capture/bake_land.cpp | 341 +++++++++++++++-------- src/shell/instrument/instrument_bake.cpp | 15 +- tests/test_bake_wire.cpp | 182 +++++++++++- 8 files changed, 491 insertions(+), 178 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 3ef3058..300e686 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -654,17 +654,16 @@ tracks-and-range-only shape. **Context (surfaced by Ψ-W2-T2, mono-collapse).** The collapse (`collapseCapturedFileToMono` / `core/capture/wav_codec::collapseToMono`) ships for every extension capture path — -offline, realtime, batch, recapture — but not for `bake_land.cpp`'s `landOne`, the -resample bake's landing function. A dead-center instrument render (the common case +offline, realtime, batch, recapture — but not for `bake_land.cpp`'s landing, the +resample bake's `prepareLanding` / `commitLanding` pair. A dead-center instrument render (the common case that motivated Ψ.6 in the first place) is exactly the dual-mono shape the predicate collapses, so an un-collapsed bake keeps paying for the second channel it doesn't need. -**Not deferred for the reason once given.** `landOne` reads the staged file into `bytes` -once (`bake_land.cpp:101`), parses its layout (`:105`), hashes it (`:126`), derives the -channel count twice (`:131`, `:178`), and writes it (`:165`) — all from that same one -buffer, so collapsing `bytes` right after the layout parse would keep the hash, the -channel count, and the written file consistent by construction; there is no ordering -hazard here to defer around. +**Not deferred for the reason once given.** `prepareLanding` reads the staged file into +`prep.bytes` once, parses its layout, hashes it and derives the channel count from that +same one buffer, and `commitLanding` writes that buffer — so collapsing it right after the +layout parse would keep the hash, the channel count, and the written file consistent by +construction; there is no ordering hazard here to defer around. **The real reason.** `bake_land.cpp` is Phase Ξ's freshly-landed surface (Ξ-W2-T1, the resample bake chain) and another team is actively remediating it. Landing @@ -677,9 +676,9 @@ however many channels the dialed sound has, and `bake_render.cpp:38` reads `sample.channelCount()` off that render rather than hardcoding 2 — a mono-programmed sound already bakes to a mono file today, with no change needed. -**Intended fix.** Once `bake_land.cpp` is quiet, call `collapseToMono` on the staged -`bytes` in `landOne` right after the layout parse (`:105`) and before the hash (`:126`), -matching the offline/realtime insertion point (post-parse, pre-identity-read). +**Intended fix.** Once `bake_land.cpp` is quiet, call `collapseToMono` on `prep.bytes` in +`prepareLanding` right after the layout parse and before the hash, matching the +offline/realtime insertion point (post-parse, pre-identity-read). **Priority / risk.** Low — a size optimization on an already-correct path, not a precision-invariant gap; the bake's dual-mono case still lands as a valid (if larger) diff --git a/src/core/wire/CLAUDE.md b/src/core/wire/CLAUDE.md index 4d11d2f..4fcb933 100644 --- a/src/core/wire/CLAUDE.md +++ b/src/core/wire/CLAUDE.md @@ -80,7 +80,7 @@ This directory owns two cross-artifact contracts specifically: - `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`. `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's write landed; `describeBakeKey` prints one line per enumerated key regardless, which is the only thing that names WHICH key — the counts cannot. +- `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. `bakeWriteLanded` is that read-back's verdict and the reason it has to exist: `SetProjExtState` returns the size of the whole extname's state, which `banks` alone keeps non-zero in every case a bake can reach, so nothing but re-reading the key can say whether THAT key took the value. - `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 73da9d6..d951a23 100644 --- a/src/core/wire/bake_wire.cpp +++ b/src/core/wire/bake_wire.cpp @@ -214,9 +214,11 @@ std::string describeBakeScan(const BakeScanTally& t) { if (t.notARequest > 0) s += "; " + std::to_string(t.notARequest) + " held something other than a request"; + // "were cleared" would assert a write this count cannot see: whether each clear + // actually took is a per-KEY read-back, and describeBakeKey is where it is said. if (t.staleCleared > 0) s += "; " + std::to_string(t.staleCleared) + - " were past the age bound and were cleared unanswered"; + " were past the age bound, so no reader was left to answer"; } s += "."; if (t.writeFailed > 0) @@ -226,9 +228,31 @@ std::string describeBakeScan(const BakeScanTally& t) { return s + "\n"; } +namespace { + +// The Land verdict's own clause. Separate function so its own fail-closed default cannot +// be swallowed by the caller's switch — an unnamed enumerator returns empty either way. +std::string describeLanding(BakeLanding landing) { + switch (landing) { + case BakeLanding::Banked: + return "landed into the bank"; + case BakeLanding::Unpersisted: + return "landed into the bank IN MEMORY ONLY -- this pass could not persist it, " + "so the project's saved bank state does not carry 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"; + case BakeLanding::Refused: + return "the landing was refused"; + } + return {}; +} + +} // namespace + std::string describeBakeKey(const std::string& key, const BakeKeyOutcome& outcome) { const std::string head = "ReaSampler resample: " + key + " -- "; - std::string answered; // set only by the two verdicts that write an outcome back + std::string clause; // set only by the verdicts that write something back switch (outcome.verdict) { case BakeScanVerdict::IgnoreUnreadable: return head + (outcome.oversized @@ -240,25 +264,36 @@ std::string describeBakeKey(const std::string& key, const BakeKeyOutcome& outcom "skipped: it holds something other than a pending request -- an answer " "nobody has collected, or a wire this build does not read.\n"; case BakeScanVerdict::ClearStale: - return head + - "cleared unanswered: it was past the age bound, so no reader was left " - "for it.\n"; + // The clear is a write like any other, so it is claimed only where it was read + // back: an unconfirmed one leaves the request standing for the next pass. + return head + (outcome.writeConfirmed + ? "past the age bound with no reader left, so it was cleared " + "unanswered.\n" + : "past the age bound with no reader left, but the clear " + "could NOT be read back -- the key still holds it and the " + "next pass will see it again.\n"); case BakeScanVerdict::RefuseWrongProject: - answered = "refused: its project tab is not the one this extension has loaded"; + clause = "refused"; break; case BakeScanVerdict::Land: - answered = outcome.landed ? "landed into the bank" : "the landing was refused"; + clause = describeLanding(outcome.landing); break; } // Fails closed on a verdict this build has no word for, the same reason // answeredOutcome exists: no -Wswitch is configured, so an appended enumerator would // otherwise fall straight into the trailing clause and print half a sentence. - if (answered.empty()) return head + "a verdict this build has no word for.\n"; - return head + answered + - (outcome.answerWritten - ? ", and the answer was written back.\n" - : ", but the answer could NOT be written back -- the instance that asked " - "will report no answer.\n"); + if (clause.empty()) return head + "a verdict this build has no word for.\n"; + std::string s = head + clause; + if (!outcome.detail.empty()) s += " (" + outcome.detail + ")"; + return s + (outcome.writeConfirmed + ? ". The answer was written back.\n" + : ". The answer could NOT be written back -- the instance that asked " + "will report no answer.\n"); +} + +bool bakeWriteLanded(const std::string& written, const std::optional& readBack) { + if (written.empty()) return !readBack || readBack->empty(); + return readBack && *readBack == written; } } // namespace reasampler::wire diff --git a/src/core/wire/bake_wire.h b/src/core/wire/bake_wire.h index 094bf7a..9d541f5 100644 --- a/src/core/wire/bake_wire.h +++ b/src/core/wire/bake_wire.h @@ -176,29 +176,52 @@ struct BakeScanTally { int answered = 0; // an outcome was QUEUED for write-back (a landing OR a // refusal) — pass-wide, not per key, and not proof the write // reached the project; `writeFailed` is that - int writeFailed = 0; // of `answered`, the ones SetProjExtState reported as not - // landed — an asking instance sees these as no answer at all + int writeFailed = 0; // of `answered`, the ones whose value could NOT be read back + // from their key afterwards — an asking instance sees these as + // no answer at all int landed = 0; // of `answered`, the ones that reached the bank }; // The console summary for one pass. Empty ONLY when the pass answered somebody AND left no -// key unanswered AND every answer's write landed — so the summary's absence means nothing -// went wrong, never that the action did not run. A silent console is NOT evidence the -// landing never ran; nothing here can observe that, and no caller may say it does. +// key unanswered AND every answer was read back from its own key — so the summary's absence +// means nothing went wrong, never that the action did not run. A silent console is NOT +// evidence the landing never ran; nothing here can observe that, and no caller may say it. std::string describeBakeScan(const BakeScanTally& tally); +// How far a `Land` verdict actually got. Only `Banked` is a landing the project still holds +// after a reload: the book lives in memory and a pass that could not persist has changed +// nothing the .rpp will carry. +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, but the pass's persist did not happen + Banked, // in the book AND persisted into the project +}; + // What ONE scanned key ended the pass in — the per-key half of the tally above, which -// counts but cannot name. Three of the fields are verdict-conditional; the phrasing +// counts but cannot name. Every field but the verdict is verdict-conditional; the phrasing // function is the one place that pairing is spelled out. struct BakeKeyOutcome { BakeScanVerdict verdict = BakeScanVerdict::IgnoreUnreadable; - bool oversized = false; // IgnoreUnreadable only: the value exceeded the read - // ceiling rather than reading back empty - bool landed = false; // Land only: the bake reached the bank - bool answerWritten = false; // Land / RefuseWrongProject only: the outcome write - // reported landing in the project + bool oversized = false; // IgnoreUnreadable only: the value exceeded the read ceiling + // rather than reading back empty + BakeLanding landing = BakeLanding::Refused; // Land only + bool writeConfirmed = false; // Land / RefuseWrongProject / ClearStale: the write this + // verdict required — an outcome, or a clear — was READ + // BACK from the key afterwards (see bakeWriteLanded) + std::string detail; // the outcome's own message, so one line per key is + // self-contained; empty for the verdicts without one }; +// Did the write we just made take? `readBack` is what the key holds afterwards (nullopt = +// absent or unreadable). This exists because SetProjExtState's return is the size of the +// WHOLE extname's state — `banks`, `project_guid` and every rsusage_ key count toward it, +// and a bake can only reach a landing in a project whose `banks` is already populated — so +// that return is non-zero whether or not THIS key took the value. Reading the key back is +// the only 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. +bool bakeWriteLanded(const std::string& written, const std::optional& readBack); + // One console line naming a key and what the pass did with it, ends in '\n'. Printed for // EVERY enumerated key, answered or not, because the counts above cannot tell an instance // which key was its own — and the key carries the asking instance's guid. diff --git a/src/shell/capture/CLAUDE.md b/src/shell/capture/CLAUDE.md index 8c7a9f0..c24db77 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: scans every open project tab for pending `rsbake_*` requests, lands the ones belonging to the project this session has loaded, 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. 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 rejected, in one `ShowConsoleMsg`. It checks `SetProjExtState`'s return on every non-empty answer — an answer that did not land is the one no-answer the tally alone cannot show. Each key is materialized before any answer is written, so no `SetProjExtState` in this action mutates a set the enumerator is still walking. The undo block is stack RAII (`UndoBlock`), and `landOne`'s whole-WAV read/hash runs under a catch that converts a throw into a refusal — neither an exception nor an early return can leave an undo block open or discard the buffered answers. 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. 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". 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. +- `bake_land` (`shell/capture`) — the EXTENSION's half of the resample chain: scans every open project tab for pending `rsbake_*` requests, lands the ones belonging to the project this session has loaded, 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. 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 rejected, in one `ShowConsoleMsg`. It PROVES every write — answer or stale-clear — by reading the key back (`wire::bakeWriteLanded`), because `SetProjExtState`'s return describes the whole extname's state and cannot speak for one key; an answer that did not land is the one no-answer the tally alone cannot show. 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. The undo block is stack RAII (`UndoBlock`), and the landing is split into a non-mutating `prepareLanding` and a mutating `commitLanding` under separate catches — a throw before anything was written is a clean refusal, a throw after it is reported as possibly partial, and neither an exception nor an early return can leave an undo block open or discard the buffered answers. 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. 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". 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. - `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. - `capture_realtime_shell` (`shell/capture`) — the async realtime-record backend surface (Q-W6 split of the former fat `capture.h`): `RealtimeRecordBackend::begin`/`tick`/`abort`, transport-driven across timer ticks (a realtime record cannot block REAPER's UI for its own duration). Deliberately shares NO interface with the offline backend — the lifecycles genuinely differ (the former `ICaptureBackend` interface was deleted in Q-W3, T4-26). diff --git a/src/shell/capture/bake_land.cpp b/src/shell/capture/bake_land.cpp index e13e047..4a991b6 100644 --- a/src/shell/capture/bake_land.cpp +++ b/src/shell/capture/bake_land.cpp @@ -81,61 +81,82 @@ const Sample* findSourceSample(const BankBook& book, const std::string& sampleId return nullptr; } -// Lands ONE request into `session`'s book, which must already be the book of the project -// `projectDir` names. Mutates book + ledger without persisting; the caller persists once for -// the batch. Every refusal path leaves the book untouched and writes no file, so a failed -// bake is invisible to the project. -BakeOutcome landOne(ReaSamplerSession& session, const std::string& projectDir, - const BakeRequest& request) { - if (projectDir.empty()) - return refuse(BakeStatus::NoProject, - "no saved project, so the bank has no location", request.generation); +// Everything a landing DECIDES, before it touches disk or book: the staged WAV read and +// hash, the source lookup, replace-vs-add, and the dedup short-circuit. Split from the +// commit below so the two halves' throws can be told apart — a prepare that threw left the +// project untouched, and only a commit that threw may have written something. +struct PreparedLanding { + std::optional settled; // a refusal or a dedup hit: nothing left to commit + BakeOutcome outcome; // the shape a successful commit completes + Sample landed; // the entry the commit indexes + std::vector bytes; // the staged file, already read + std::string bankId; + std::string destDir; + std::string destPath; + bool replace = false; +}; - const std::vector bytes = util::readFileBytes(request.stagedFilePath); - if (bytes.empty()) - return refuse(BakeStatus::StagedMissing, "the staged render was unreadable", - request.generation); - const WavLayout layout = parseWavLayout(bytes); - if (!layout.valid || layout.frameCount() == 0) - return refuse(BakeStatus::StagedMissing, "the staged render is not a usable WAV", - request.generation); +PreparedLanding prepareLanding(ReaSamplerSession& session, const std::string& projectDir, + const BakeRequest& request) { + PreparedLanding prep; + if (projectDir.empty()) { + prep.settled = refuse(BakeStatus::NoProject, + "no saved project, so the bank has no location", + request.generation); + return prep; + } + + prep.bytes = util::readFileBytes(request.stagedFilePath); + if (prep.bytes.empty()) { + prep.settled = refuse(BakeStatus::StagedMissing, "the staged render was unreadable", + request.generation); + return prep; + } + const WavLayout layout = parseWavLayout(prep.bytes); + if (!layout.valid || layout.frameCount() == 0) { + prep.settled = refuse(BakeStatus::StagedMissing, + "the staged render is not a usable WAV", request.generation); + return prep; + } BankBook& book = session.book(); - std::string bankId; - const Sample* source = findSourceSample(book, request.sourceSampleId, bankId); - if (!source) - return refuse(BakeStatus::NoSource, "the resampled capture is not in any bank", - request.generation); - // Copied, not aliased: every mutation below invalidates the book's pointers. + const Sample* source = findSourceSample(book, request.sourceSampleId, prep.bankId); + if (!source) { + prep.settled = refuse(BakeStatus::NoSource, + "the resampled capture is not in any bank", request.generation); + return prep; + } + // Copied, not aliased: every mutation the commit makes invalidates the book's pointers. const Sample sourceCopy = *source; const tracking::Landing landing = tracking::resampleLanding( session.tiedUsageFor(request.sourceRelativePath, request.ownUsageKey)); - const bool replace = (landing == tracking::Landing::Replace); + prep.replace = (landing == tracking::Landing::Replace); // WAV-aware hash: the bank's dedup key, and — on the add path only — the reason a // byte-identical bake yields no second entry. Replace never dedups, matching // updateSampleInPlace's own contract: an in-place refresh is not an insert. - const std::string contentHash = hashWavContent(bytes); + const std::string contentHash = hashWavContent(prep.bytes); - BakeOutcome out; - out.generation = request.generation; - out.rootNote = request.rootNote; - out.channelCount = static_cast(layout.channelCount); - out.replaced = replace; + prep.outcome.generation = request.generation; + prep.outcome.rootNote = request.rootNote; + prep.outcome.channelCount = static_cast(layout.channelCount); + prep.outcome.replaced = prep.replace; - if (!replace && !contentHash.empty()) { - if (const model::BankModel* index = book.index(bankId)) { + if (!prep.replace && !contentHash.empty()) { + if (const model::BankModel* index = book.index(prep.bankId)) { if (const Sample* existing = index->findByHash(contentHash)) { // Dedup BEFORE the disk write: this bake is bytes the bank already holds, // so it re-points at that entry rather than depositing an unreferenced // twin for the prune to reclaim later. + BakeOutcome out = prep.outcome; out.status = BakeStatus::Ok; out.sampleId = existing->id; out.relativePath = existing->relativePath; out.displayName = existing->displayName; out.message = "identical to an existing capture -- pointed at it"; - return out; + prep.settled = std::move(out); + return prep; } } } @@ -151,61 +172,128 @@ BakeOutcome landOne(ReaSamplerSession& session, const std::string& projectDir, const std::string stem = sourceCopy.displayName.empty() ? std::string("resample") : sourceCopy.displayName; const BankPaths paths = deriveBankPaths(projectDir, stem, uniqueTag); + prep.destDir = paths.absoluteDir; + prep.destPath = paths.absoluteDir + "/" + paths.fileName; - std::error_code ec; - fs::create_directories(paths.absoluteDir, ec); // idempotent; the write reports failure - const std::string destPath = paths.absoluteDir + "/" + paths.fileName; - if (!writeFileBytes(destPath, bytes)) - return refuse(BakeStatus::Failed, "could not write the bake into the bank folder", - request.generation); - - Sample landed; // Replace keeps the entry's identity and its slot — the sound iterated, it did not // become a different capture. The superseded FILE is untouched: it stays on disk, // unreferenced, until a prune reclaims it, which is the iterate loop's recovery floor. - landed.id = replace ? sourceCopy.id - : ("bake-" + uniqueTag + "-" + paths.fileName); - landed.displayName = - replace ? sourceCopy.displayName : nextIterationName(sourceCopy.displayName); - landed.relativePath = paths.relativePath; // project-relative (invariant) - landed.channelCount = static_cast(layout.channelCount); - landed.sampleRate = static_cast(layout.sampleRate); - landed.lengthSeconds = + prep.landed.id = + prep.replace ? sourceCopy.id : ("bake-" + uniqueTag + "-" + paths.fileName); + prep.landed.displayName = + prep.replace ? sourceCopy.displayName : nextIterationName(sourceCopy.displayName); + prep.landed.relativePath = paths.relativePath; // project-relative (invariant) + prep.landed.channelCount = static_cast(layout.channelCount); + prep.landed.sampleRate = static_cast(layout.sampleRate); + prep.landed.lengthSeconds = layout.sampleRate ? static_cast(layout.frameCount()) / static_cast(layout.sampleRate) : 0.0; - landed.rootNote = request.rootNote; // rendered AT root — that is what makes it survive - landed.tier = model::Tier::Scratch; - landed.contentHash = contentHash; - landed.createdTimestamp = nowSec; + // Rendered AT root — that is what makes it survive. + prep.landed.rootNote = request.rootNote; + prep.landed.tier = model::Tier::Scratch; + prep.landed.contentHash = contentHash; + prep.landed.createdTimestamp = nowSec; // The lineage seed recordCreated reads: the ledger's parent chain is what makes a // repeated bake readable as one iteration chain. - landed.provenance = model::Provenance{sourceCopy.id, std::string{}}; + prep.landed.provenance = model::Provenance{sourceCopy.id, std::string{}}; + return prep; +} - const bool indexed = replace - ? book.updateSampleInPlace(sourceCopy.id, landed) - : (book.index(bankId) && - book.index(bankId)->add(landed) == AddResult::Added); +// The mutating half. From its first successful write a file exists and the book can change, +// so a throw out of THIS function is not invisible to the project — which is why its caller +// reports it differently from a prepare that threw. Its own refusals withdraw the file they +// wrote, best-effort. Mutates book + ledger without persisting; the caller persists once for +// the batch. +BakeOutcome commitLanding(ReaSamplerSession& session, PreparedLanding& prep) { + std::error_code ec; + fs::create_directories(prep.destDir, ec); // idempotent; the write reports failure + if (!writeFileBytes(prep.destPath, prep.bytes)) { + // A failed write can still have created the file before failing; withdraw it for + // the same reason the refused index below does. + fs::remove(prep.destPath, ec); + return refuse(BakeStatus::Failed, "could not write the bake into the bank folder", + prep.outcome.generation); + } + + BankBook& book = session.book(); + // On the replace path prepareLanding set `landed.id` to the source's own id, so this is + // the entry being refreshed in place. + const bool indexed = + prep.replace ? book.updateSampleInPlace(prep.landed.id, prep.landed) + : (book.index(prep.bankId) && + book.index(prep.bankId)->add(prep.landed) == AddResult::Added); if (!indexed) { // Self-cleanup of a file this call wrote seconds ago and never indexed — the // carve-out shell/persist/CLAUDE.md states, not prune's authority over the bank's // known bytes. Leaving it would deposit an untracked orphan per refused bake. - fs::remove(destPath, ec); + fs::remove(prep.destPath, ec); return refuse(BakeStatus::IndexRejected, - replace ? "the bank refused the replacement" - : "the bank refused the new capture", - request.generation); + prep.replace ? "the bank refused the replacement" + : "the bank refused the new capture", + prep.outcome.generation); } - session.recordCreated(landed, tracking::OriginKind::Capture); + session.recordCreated(prep.landed, tracking::OriginKind::Capture); + BakeOutcome out = prep.outcome; out.status = BakeStatus::Ok; - out.sampleId = landed.id; - out.relativePath = landed.relativePath; - out.displayName = landed.displayName; - out.message = replace ? "replaced the bank entry" : "added as a distinct capture"; + out.sampleId = prep.landed.id; + out.relativePath = prep.landed.relativePath; + out.displayName = prep.landed.displayName; + out.message = prep.replace ? "replaced the bank entry" : "added as a distinct capture"; return out; } +// One request landed, with the two failure truths kept apart: a throw out of the prepare +// half is a refusal that left the project untouched, while a throw out of the commit half +// may have written a file and changed the book — calling that one "refused" would tell the +// user nothing happened when something did. +struct LandingAttempt { + BakeOutcome outcome; + bool partial = false; // the commit threw: a file and/or an entry may exist + bool changedBook = false; // the book gained or refreshed an entry, unpersisted so far +}; + +LandingAttempt attemptLanding(ReaSamplerSession& session, const std::string& projectDir, + const BakeRequest& request) { + LandingAttempt attempt; + PreparedLanding prep; + try { + prep = prepareLanding(session, projectDir, request); + } catch (const std::exception& e) { + attempt.outcome = + refuse(BakeStatus::Failed, + std::string("the landing failed before it wrote anything: ") + e.what(), + request.generation); + return attempt; + } catch (...) { + attempt.outcome = + refuse(BakeStatus::Failed, + "the landing failed before it wrote anything, for an unknown reason", + request.generation); + return attempt; + } + if (prep.settled) { + attempt.outcome = std::move(*prep.settled); + return attempt; // a refusal, or a dedup hit that changed nothing + } + try { + attempt.outcome = commitLanding(session, prep); + attempt.changedBook = attempt.outcome.status == BakeStatus::Ok; + } catch (const std::exception& e) { + attempt.partial = true; + attempt.outcome = refuse(BakeStatus::Failed, + std::string("the landing failed while writing: ") + e.what(), + request.generation); + } catch (...) { + attempt.partial = true; + attempt.outcome = refuse(BakeStatus::Failed, + "the landing failed while writing, for an unknown reason", + request.generation); + } + return attempt; +} + // Every "rsbake_*" key in one project, keys only — the value can outgrow // EnumProjExtState's fixed buffer, so it is read separately by the growing reader. std::vector pendingBakeKeys(ReaProject* proj) { @@ -241,6 +329,15 @@ KeyRead readKey(ReaProject* proj, const std::string& key) { return out; } +// Writes one value under `key` and PROVES it by reading the key back. SetProjExtState's own +// return cannot do that: it is the size of the whole extname's state, which `banks` alone +// keeps non-zero in every case a bake can reach. One extra read per written key, on a cold +// path. An empty `value` is a clear. +bool writeKeyVerified(ReaProject* proj, const std::string& key, const std::string& value) { + SetProjExtState(proj, kProjExtNamespace(), key.c_str(), value.c_str()); + return wire::bakeWriteLanded(value, readKey(proj, key).value); +} + struct OpenProject { ReaProject* proj = nullptr; std::string dir; // the project's own directory; empty for a never-saved project @@ -260,18 +357,20 @@ std::vector openProjects() { return out; } -// One scanned key: what the pass decided about it, and the answer to write back after the -// undo block closes. +// One scanned key: what the pass decided about it, and what to write back after the undo +// block closes. The outcome is kept UNENCODED until then, because the pass's persist runs +// after the whole scan and an Ok the project never took must not be sent as one. struct ScannedKey { + enum class Write { None, Clear, Answer }; + ReaProject* proj = nullptr; std::string key; - bool writesAnswer = false; // false = an Ignore verdict, nothing to write - std::string answerWire; // with writesAnswer: empty = clear the key instead - std::string console; // the outcome's own sentence; empty = nothing to print + Write write = Write::None; // None = an Ignore verdict, nothing to write + BakeOutcome outcome; // Write::Answer only wire::BakeKeyOutcome report; }; -// Undo_BeginBlock2/EndBlock2 as stack RAII, the FxBypassGuard discipline: landOne reads a +// Undo_BeginBlock2/EndBlock2 as stack RAII, the FxBypassGuard discipline: a landing reads a // whole WAV and hashes it, so a bad_alloc between the two calls would otherwise leave an // open undo block in the user's project. Closing with an empty description records no undo // point, which is what every path that landed nothing wants anyway. @@ -313,9 +412,10 @@ 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 - // The scan allocates outside landOne's own guard too (readKey grows a buffer toward - // 16 MB, every container here allocates), and a throw that escaped would discard the - // answers already buffered — the exact no-answer-with-a-silent-console this action + bool persisted = false; // the pass's ONE persist actually happened + // The scan allocates outside attemptLanding's own guards too (readKey grows a buffer + // toward 16 MB, every container here allocates), and a throw that escaped would discard + // the answers already buffered — the exact no-answer-with-a-silent-console this action // exists to make impossible. UndoBlock is inside the try, so unwinding still closes it. try { UndoBlock undo; @@ -358,49 +458,36 @@ void RunResampleBake(ReaSamplerSession& session) { } if (verdict == wire::BakeScanVerdict::ClearStale) { ++tally.staleCleared; - entry.writesAnswer = true; // empty answerWire = clear the key + entry.write = ScannedKey::Write::Clear; scanned.push_back(std::move(entry)); continue; } - BakeOutcome outcome; if (verdict == wire::BakeScanVerdict::RefuseWrongProject) { - outcome = + entry.outcome = refuse(BakeStatus::WrongProject, "this bake's project tab is not the one the extension has " "loaded -- focus that tab and try again", request->generation); } else { - // landOne reads and hashes a whole WAV. A throw here would otherwise - // discard every answer buffered so far and leave each asking instance - // with a no-answer it cannot explain; converting it to a refusal keeps - // the pass and its report intact. - try { - outcome = landOne(session, open.dir, *request); - } catch (const std::exception& e) { - outcome = refuse(BakeStatus::Failed, - std::string("the landing failed: ") + e.what(), - request->generation); - } catch (...) { - outcome = refuse(BakeStatus::Failed, - "the landing failed for an unknown reason", - request->generation); - } - if (outcome.status == BakeStatus::Ok) ++tally.landed; + LandingAttempt attempt = attemptLanding(session, open.dir, *request); + if (attempt.outcome.status == BakeStatus::Ok) ++tally.landed; + // Unpersisted rather than Banked for anything that changed the book: the + // pass's persist has not run yet, and the write loop is where the + // upgrade is earned. A dedup hit changed nothing, so its landing is not + // this pass's to lose. + if (attempt.partial) + entry.report.landing = wire::BakeLanding::Partial; + else if (attempt.outcome.status != BakeStatus::Ok) + entry.report.landing = wire::BakeLanding::Refused; + else + entry.report.landing = attempt.changedBook + ? wire::BakeLanding::Unpersisted + : wire::BakeLanding::Banked; + entry.outcome = std::move(attempt.outcome); } ++tally.answered; // QUEUED, not written — the write loop below judges that - entry.report.landed = outcome.status == BakeStatus::Ok; - // A WrongProject request left sitting in a tab this call did not come from - // prints again on every OTHER tab's bake, since the scan revisits every - // open project each time. Only a refusal in REAPER's active tab is likely - // to be fresh feedback to a user who just clicked bake; every other one is - // a rescan repeat. The per-key line below is printed either way. - if (outcome.status != BakeStatus::Ok && - (outcome.status != BakeStatus::WrongProject || activeTab)) { - entry.console = "ReaSampler resample: " + outcome.message + ".\n"; - } - entry.writesAnswer = true; - entry.answerWire = wire::encodeBakeOutcome(outcome); + entry.write = ScannedKey::Write::Answer; scanned.push_back(std::move(entry)); } } @@ -409,14 +496,15 @@ void RunResampleBake(ReaSamplerSession& session) { // A bake changes what a live instance would play, so the generation bump rides // the persist — every other open instance refreshes hands-free. session.bumpBankGeneration(); - if (session.saveToActiveProject()) + persisted = session.saveToActiveProject(); + if (persisted) undo.recordPoint("ReaSampler: resample bake into bank", UNDO_STATE_MISCCFG); } } catch (...) { aborted = "ReaSampler resample: the landing pass stopped early on an internal failure. " - "The answers it had already prepared are written below; any key it had not " - "reached yet went unanswered and is not named at all.\n"; + "The answers it had already prepared are written below; any key whose answer it " + "had not prepared yet went unanswered and is not named at all.\n"; } // OUTSIDE the undo block on purpose: the answer is transient handshake state, and an @@ -424,20 +512,33 @@ void RunResampleBake(ReaSamplerSession& session) { // which the following bake would then re-land against a temp file that is long gone. std::string console = aborted; for (ScannedKey& entry : scanned) { - if (entry.writesAnswer) { - // SetProjExtState returns the size of the extname's state, so storing a - // non-empty value necessarily returns > 0 and <= 0 means the write did not - // land. A deliberate clear shrinks the state and can legitimately return 0 — - // the same reading reaper_bridge's writeGuarded applies to the other end. - const int rv = SetProjExtState(entry.proj, kProjExtNamespace(), - entry.key.c_str(), entry.answerWire.c_str()); - entry.report.answerWritten = entry.answerWire.empty() || rv > 0; - if (!entry.report.answerWritten) ++tally.writeFailed; + if (entry.write != ScannedKey::Write::None) { + std::string value; // empty = clear the key + if (entry.write == ScannedKey::Write::Answer) { + // Where a landing earns the word: the persist ran 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. + if (entry.report.landing == wire::BakeLanding::Unpersisted) { + if (persisted) + entry.report.landing = wire::BakeLanding::Banked; + else + entry.outcome = refuse(BakeStatus::Failed, + "the bake reached the bank in memory, but " + "this pass could not persist it, so the " + "project's saved bank state does not carry it", + entry.outcome.generation); + } + value = wire::encodeBakeOutcome(entry.outcome); + entry.report.detail = entry.outcome.message; + } + entry.report.writeConfirmed = writeKeyVerified(entry.proj, entry.key, value); + if (!entry.report.writeConfirmed && entry.write == ScannedKey::Write::Answer) + ++tally.writeFailed; } // Unconditional, including on success: the tally counts keys but cannot name them, // and the key name is the only thing that tells an instance which line is its own. console += wire::describeBakeKey(entry.key, entry.report); - console += entry.console; } console += wire::describeBakeScan(tally); if (!console.empty()) ShowConsoleMsg(console.c_str()); diff --git a/src/shell/instrument/instrument_bake.cpp b/src/shell/instrument/instrument_bake.cpp index 85d115d..af6e94d 100644 --- a/src/shell/instrument/instrument_bake.cpp +++ b/src/shell/instrument/instrument_bake.cpp @@ -196,9 +196,9 @@ BakeChainResult runBake(ReaSamplerProcessor& processor) { // What the key holds now is the only evidence this side gets, and each of the five // non-answers is a different thing to go fix — collapsing them into one sentence is - // what made a stale install indistinguishable from a refusal. The three that are about - // the KEY name it, because the landing prints one console line per key it scanned and - // the key is what correlates the two in a multi-instance session. + // what made a stale install indistinguishable from a refusal. All five name the KEY, + // because the landing prints one console line per key it scanned and the key is what + // correlates the two in a multi-instance session. const wire::BakeAnswer answer = wire::classifyBakeAnswer(bridge.readReasamplerExtState(key), request); switch (answer.kind) { @@ -210,9 +210,9 @@ BakeChainResult runBake(ReaSamplerProcessor& processor) { "landing action prints one REAPER console line per key it " "scanned; look for that key name there"); case wire::BakeAnswerKind::Undecodable: - return fail( - "the extension answered in a format this plugin does not read -- the " - "extension and ReaSampler 9000 are from different builds"); + return fail("the value under " + key + + " is neither a request nor an answer this build can read -- the " + "extension and ReaSampler 9000 may be from different builds"); case wire::BakeAnswerKind::Cleared: return fail("the bake key " + key + " came back empty -- either the request was cleared before an " @@ -225,7 +225,8 @@ BakeChainResult runBake(ReaSamplerProcessor& processor) { "unexpected given the bake's single-threaded call flow; if this " "recurs, note the exact steps and file it"); case wire::BakeAnswerKind::ForeignOutcome: - return fail("the extension answered a different bake request"); + return fail("the answer under " + key + + " is for a different bake request than this one"); } // wire::answeredOutcome is the guard, not switch exhaustiveness alone: the switch // above has no `default`, so a future BakeAnswerKind enumerator it doesn't yet handle diff --git a/tests/test_bake_wire.cpp b/tests/test_bake_wire.cpp index e215ab9..acea970 100644 --- a/tests/test_bake_wire.cpp +++ b/tests/test_bake_wire.cpp @@ -7,7 +7,8 @@ // break a delimiter-based format; the refusals every house wire record shares (wrong tag, // truncation, trailing garbage, a swapped record kind); an unrecognized status integer // degrading to Failed rather than to Ok; the action lookup name's leading underscore and -// its channel fork; and the two key classifiers each end reads the shared key through. +// its channel fork; the two key classifiers each end reads the shared key through; and the +// write-back verdict, driven by a modelled key store that accepts or drops the write. #include "../src/core/wire/bake_wire.h" @@ -15,6 +16,7 @@ #include #include +#include #include #include @@ -482,7 +484,9 @@ int main() { CHECK(other.find("1 held something other than a request") != std::string::npos); CHECK(other.find("could not be read back") == std::string::npos); - // Cleared as stale: an answer was never written, so this too must report. + // Cleared as stale: an answer was never written, so this too must report. The + // summary may NOT say the clears were written — whether each one took is a per-key + // read-back, and only describeBakeKey has seen it. BakeScanTally stale; stale.tabsScanned = 1; stale.keysFound = 1; @@ -490,6 +494,7 @@ int main() { stale.staleCleared = 1; const std::string aged = describeBakeScan(stale); CHECK(aged.find("past the age bound") != std::string::npos); + CHECK(aged.find("were cleared") == std::string::npos); // activeTabKeys is REAPER's ACTIVE tab and nothing more. It cannot discriminate the // multi-tab mis-target — a genuine background-tab request classifies as @@ -515,40 +520,77 @@ int main() { BakeKeyOutcome landed; landed.verdict = BakeScanVerdict::Land; - landed.landed = true; - landed.answerWritten = true; + landed.landing = BakeLanding::Banked; + landed.writeConfirmed = true; + landed.detail = "added as a distinct capture"; const std::string ok = describeBakeKey(key, landed); CHECK(ok.find(key) != std::string::npos); CHECK(ok.find("landed into the bank") != std::string::npos); - CHECK(ok.find("the answer was written back") != std::string::npos); + CHECK(ok.find("The answer was written back") != std::string::npos); CHECK(ok.back() == '\n'); + // The outcome's own reason rides the SAME line: one self-contained line per key, + // 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); // A landing whose answer write was REJECTED — the state the instrument reads as // Unanswered. This line is the only place it is ever named. BakeKeyOutcome lost = landed; - lost.answerWritten = false; + lost.writeConfirmed = false; const std::string dropped = describeBakeKey(key, lost); CHECK(dropped.find("landed into the bank") != std::string::npos); CHECK(dropped.find("could NOT be written back") != std::string::npos); CHECK(dropped.find("will report no answer") != std::string::npos); + // The four landing states are four different lines: a bake the project never + // persisted, and one that threw mid-write, may not read as a clean success or as a + // clean refusal. + BakeKeyOutcome unpersisted = landed; + unpersisted.landing = BakeLanding::Unpersisted; + const std::string memoryOnly = describeBakeKey(key, unpersisted); + CHECK(memoryOnly.find("IN MEMORY ONLY") != std::string::npos); + CHECK(memoryOnly.find("could not persist it") != std::string::npos); + CHECK(memoryOnly != ok); + + BakeKeyOutcome partial = landed; + partial.landing = BakeLanding::Partial; + partial.detail = "the landing failed while writing: bad allocation"; + const std::string half = describeBakeKey(key, partial); + CHECK(half.find("after it had begun writing") != std::string::npos); + CHECK(half.find("may have left a file") != std::string::npos); + CHECK(half.find("the landing was refused") == std::string::npos); + BakeKeyOutcome bankRefused; bankRefused.verdict = BakeScanVerdict::Land; - bankRefused.landed = false; - bankRefused.answerWritten = true; - CHECK(describeBakeKey(key, bankRefused).find("the landing was refused") != - std::string::npos); + bankRefused.landing = BakeLanding::Refused; + bankRefused.writeConfirmed = true; + bankRefused.detail = "the bank refused the new capture"; + const std::string refusedLine = describeBakeKey(key, bankRefused); + CHECK(refusedLine.find("the landing was refused") != std::string::npos); + // The refusal's REASON is what the keyed line used to lack entirely. + CHECK(refusedLine.find("(the bank refused the new capture)") != std::string::npos); BakeKeyOutcome wrongTab; wrongTab.verdict = BakeScanVerdict::RefuseWrongProject; - wrongTab.answerWritten = true; + wrongTab.writeConfirmed = true; + wrongTab.detail = + "this bake's project tab is not the one the extension has loaded -- focus that " + "tab and try again"; const std::string refused = describeBakeKey(key, wrongTab); - CHECK(refused.find("not the one this extension has loaded") != std::string::npos); - CHECK(refused.find("the answer was written back") != std::string::npos); + CHECK(refused.find("not the one the extension has loaded") != std::string::npos); + CHECK(refused.find("focus that tab and try again") != std::string::npos); + CHECK(refused.find("The answer was written back") != std::string::npos); + // A clear is a write too: it may be claimed only where it was read back. BakeKeyOutcome cleared; cleared.verdict = BakeScanVerdict::ClearStale; + cleared.writeConfirmed = true; CHECK(describeBakeKey(key, cleared).find("cleared unanswered") != std::string::npos); + BakeKeyOutcome clearLost = cleared; + clearLost.writeConfirmed = false; + const std::string stuck = describeBakeKey(key, clearLost); + CHECK(stuck.find("the clear could NOT be read back") != std::string::npos); + CHECK(stuck.find("next pass will see it again") != std::string::npos); + CHECK(stuck.find("it was cleared") == std::string::npos); BakeKeyOutcome notRequest; notRequest.verdict = BakeScanVerdict::IgnoreNotARequest; @@ -573,6 +615,105 @@ int main() { CHECK(describeBakeKey("rsbake_ffff0000", landed) != ok); } + // --- A rejected write is REACHABLE, and it is what the tally and the line come from --- + // The shell used to judge this by SetProjExtState's return, which is the size of the + // WHOLE extname's state — `banks` alone keeps that non-zero in every case a bake can + // reach, so `writeFailed` could not be produced at all and the sentence for it was dead + // code. The verdict is now the read-back below, which a store that drops the write does + // produce. The store is modelled here; the shell binds these same two calls to + // SetProjExtState and its existing grow-loop GetProjExtState read. + { + struct FakeKeyStore { + std::map values; + bool dropWrites = false; + + void write(const std::string& k, const std::string& v) { + if (dropWrites) return; // REAPER rejected it + if (v.empty()) values.erase(k); // SetProjExtState("") deletes + else values[k] = v; + } + std::optional read(const std::string& k) const { + const auto it = values.find(k); + return it == values.end() ? std::nullopt + : std::optional(it->second); + } + }; + + const std::string key = "rsbake_0123abcd"; + BakeRequest pending; + pending.instanceGuid = "0123abcd"; + pending.generation = 1893456000; + BakeOutcome landedOk; + landedOk.status = BakeStatus::Ok; + landedOk.sampleId = "bake-1"; + landedOk.message = "added as a distinct capture"; + landedOk.generation = pending.generation; + const std::string answer = encodeBakeOutcome(landedOk); + + // Rejected: the key still holds the request the instrument left there. + FakeKeyStore rejecting; + rejecting.values[key] = encodeBakeRequest(pending); + rejecting.dropWrites = true; + rejecting.write(key, answer); + CHECK(!bakeWriteLanded(answer, rejecting.read(key))); + // ...which is exactly what the ASKING end reads as a no-answer. The two classifiers + // agree about this one state, which is why the extension has to name it. + CHECK(classifyBakeAnswer(rejecting.read(key), pending).kind == + BakeAnswerKind::Unanswered); + + // The shell's write loop from that verdict through to both strings it prints. + BakeScanTally tally; + tally.tabsScanned = 1; + tally.keysFound = 1; + tally.activeTabKeys = 1; + tally.answered = 1; + tally.landed = 1; + BakeKeyOutcome report; + report.verdict = BakeScanVerdict::Land; + report.landing = BakeLanding::Banked; + report.detail = landedOk.message; + report.writeConfirmed = bakeWriteLanded(answer, rejecting.read(key)); + if (!report.writeConfirmed) ++tally.writeFailed; + CHECK(tally.writeFailed == 1); + CHECK(describeBakeKey(key, report).find("could NOT be written back") != + std::string::npos); + CHECK(describeBakeScan(tally).find("1 answer could not be written back") != + std::string::npos); + + // The same store ACCEPTING the write: the verdict flips, the tally stays at zero and + // the summary goes silent — so neither answer above is a constant. + FakeKeyStore accepting; + accepting.values[key] = encodeBakeRequest(pending); + accepting.write(key, answer); + CHECK(bakeWriteLanded(answer, accepting.read(key))); + CHECK(classifyBakeAnswer(accepting.read(key), pending).kind == + BakeAnswerKind::Answered); + BakeScanTally clean = tally; + clean.writeFailed = 0; + CHECK(describeBakeScan(clean).empty()); + + // A CLEAR is proven the other way round: it lands as an ABSENT key, and a dropped + // clear leaves the stale request standing for the next pass to find. + FakeKeyStore clearing; + clearing.values[key] = encodeBakeRequest(pending); + clearing.write(key, ""); + CHECK(bakeWriteLanded("", clearing.read(key))); + FakeKeyStore clearDropped; + clearDropped.values[key] = encodeBakeRequest(pending); + clearDropped.dropWrites = true; + clearDropped.write(key, ""); + CHECK(!bakeWriteLanded("", clearDropped.read(key))); + + // Byte equality is the claim the line makes: a truncated or foreign value under the + // key is not the answer we wrote, and an unreadable key proves nothing at all. + CHECK(!bakeWriteLanded(answer, std::optional( + answer.substr(0, answer.size() - 1)))); + CHECK(!bakeWriteLanded(answer, std::optional( + encodeBakeRequest(pending)))); + CHECK(!bakeWriteLanded(answer, std::nullopt)); + CHECK(!bakeWriteLanded("", std::optional("leftover"))); + } + // --- 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. @@ -623,9 +764,22 @@ int main() { writeFailed.status = BakeStatus::Failed; writeFailed.message = "could not write the bake into the bank folder"; + // A landing the project would not persist is answered as a FAILURE, not as the Ok + // it was on its way to being: the entry exists in memory only, so an instance that + // adopted it would be pointing at something no reload will have. + BakeOutcome unpersisted = noProject; + unpersisted.status = BakeStatus::Failed; + unpersisted.message = + "the bake reached the bank in memory, but this pass could not persist it, so " + "the project's saved bank state does not carry it"; + + BakeOutcome partial = noProject; + partial.status = BakeStatus::Failed; + partial.message = "the landing failed while writing: bad allocation"; + for (const BakeOutcome& emitted : {added, replaced, deduped, noProject, stagedMissing, noSource, indexRejected, - wrongProject, writeFailed}) { + wrongProject, writeFailed, unpersisted, partial}) { const auto back = decodeBakeOutcome(encodeBakeOutcome(emitted)); CHECK(back.has_value()); CHECK(back.has_value() && *back == emitted);