Fix the bake scan report's unsound absence-as-signal claim

Gate the summary on any unanswered key and on SetProjExtState's return, print one line per scanned key, and make the undo block stack RAII.
2026-08-02 08:34:59 -04:00
parent 2f8e88880e
commit 6cabbd0dd4
8 changed files with 420 additions and 150 deletions
@@ -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_<guid>`): 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, is what separates an extension that never ran the landing from one that refused) 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` are that same reading counted and spoken — the rationale lives at the type. The report is empty whenever a pass answered anybody, which makes its ABSENCE from the console evidence too: no line means the landing action never ran.
+- `bake_wire` — the resample bake's request/outcome pair on ONE per-instance key (`rsbake_<guid>`): 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.
 - `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.

@@ -187,30 +187,78 @@ BakeScanVerdict classifyBakeScan(const BakeScanContext& session, const BakeScanK
 }

 std::string describeBakeScan(const BakeScanTally& t) {
-    if (t.answered > 0) return {};
+    // `answered` is pass-wide and counts a QUEUED write, so it cannot stand alone: a pass
+    // that answered somebody else's key while skipping ours, or that queued our answer and
+    // had the write rejected, would otherwise print nothing at all and leave the asking
+    // instance with a no-answer and a silent console.
+    const int unanswered = t.unreadable + t.notARequest + t.staleCleared;
+    if (t.answered > 0 && unanswered == 0 && t.writeFailed == 0) return {};

    std::string s = "ReaSampler resample: the landing action ran and scanned " +
-                    countOf(t.tabsScanned, "project tab") + ", and answered nothing. ";
-    if (t.keysFound == 0) {
-        s += "No pending bake request was visible to it at all -- if ReaSampler 9000 "
-             "reported publishing one, the plugin and the extension are not reading the "
-             "same project's ext state.";
-        return s + "\n";
-    }
+                    countOf(t.tabsScanned, "project tab") + ". ";
+    if (t.keysFound == 0)
+        return s +
+               "No rsbake_ key was visible to it in any of them. That fits a request "
+               "published into a project this scan did not enumerate, a key cleared "
+               "before the scan reached it, or a key set this session that the "
+               "enumeration cannot yet see -- this pass cannot tell which apart.\n";

-    s += "It found " + countOf(t.keysFound, "pending request key") + ", " +
-         std::to_string(t.activeTabKeys) + " of them in the active tab";
-    if (t.unreadable > 0)
-        s += "; " + std::to_string(t.unreadable) + " could not be read back";
-    if (t.notARequest > 0)
-        s += "; " + std::to_string(t.notARequest) + " held something other than a request";
-    if (t.staleCleared > 0)
-        s += "; " + std::to_string(t.staleCleared) +
-             " were past the age bound and were cleared unanswered";
+    s += "It found " + countOf(t.keysFound, "rsbake_ key") +
+         " (every such key, not pending requests only), " +
+         std::to_string(t.activeTabKeys) + " of them in REAPER's active tab. It answered " +
+         std::to_string(t.answered);
+    if (unanswered > 0) {
+        s += " and left " + std::to_string(unanswered) + " unanswered";
+        if (t.unreadable > 0)
+            s += "; " + std::to_string(t.unreadable) + " could not be read back";
+        if (t.notARequest > 0)
+            s += "; " + std::to_string(t.notARequest) +
+                 " held something other than a request";
+        if (t.staleCleared > 0)
+            s += "; " + std::to_string(t.staleCleared) +
+                 " were past the age bound and were cleared unanswered";
+    }
    s += ".";
-    if (t.activeTabKeys == 0)
-        s += " The tab this bake was fired against held none of them.";
+    if (t.writeFailed > 0)
+        s += " " + countOf(t.writeFailed, "answer") +
+             " could not be written back into the project, so whoever asked for them sees "
+             "no answer at all.";
    return s + "\n";
 }

+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
+    switch (outcome.verdict) {
+        case BakeScanVerdict::IgnoreUnreadable:
+            return head + (outcome.oversized
+                               ? "skipped: its value was too large to read back whole.\n"
+                               : "skipped: the enumerator listed it but it read back "
+                                 "empty.\n");
+        case BakeScanVerdict::IgnoreNotARequest:
+            return head +
+                   "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";
+        case BakeScanVerdict::RefuseWrongProject:
+            answered = "refused: its project tab is not the one this extension has loaded";
+            break;
+        case BakeScanVerdict::Land:
+            answered = outcome.landed ? "landed into the bank" : "the landing was refused";
+            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");
+}
+
 }  // namespace reasampler::wire
@@ -85,12 +85,14 @@ std::optional<BakeOutcome> decodeBakeOutcome(const std::string& wire);
 // evidence either side gets. The two classifiers below are that reading, stated once.

 // What the instrument found under its own key once invokeExtensionAction returned. The
-// distinction that matters: `Unanswered` is the extension never having read the key at
-// all, which is a DIFFERENT fault from every refusal — a refusal is an outcome.
+// distinction that matters: `Unanswered` is nothing having WRITTEN over the key, which is
+// a DIFFERENT fault from every refusal — a refusal is an outcome. It does NOT mean the
+// landing never read the key: a scan that skipped this key, and a scan whose answer write
+// was rejected, both leave the request sitting here identically.
 enum class BakeAnswerKind {
    Answered,        // a decodable outcome echoing this request's generation
    ForeignOutcome,  // a decodable outcome, but for another generation
-    Unanswered,      // this request, unchanged: nothing on the extension side read it
+    Unanswered,      // this request, unchanged: nothing wrote an outcome over it
    ForeignRequest,  // a request that is not ours — another instance shares this key
    Cleared,         // the key holds nothing — absent, explicitly cleared, or a read
                     // failure at the bridge; these are not distinguishable from here
@@ -161,20 +163,45 @@ BakeScanVerdict classifyBakeScan(const BakeScanContext& session, const BakeScanK
 // request, untouched — and only these counts say which of them happened.
 struct BakeScanTally {
    int tabsScanned = 0;
-    int keysFound = 0;      // `rsbake_*` keys enumerated across every open tab
-    int activeTabKeys = 0;  // of those, in the tab the action was fired against
+    int keysFound = 0;      // EVERY `rsbake_*` key enumerated across every open tab —
+                            // uncollected outcomes and unreadable values included, not
+                            // pending requests only
+    int activeTabKeys = 0;  // of those, in REAPER's active tab. NOT "the tab the bake was
+                            // fired against": the action is invoked with the instance's
+                            // own project as `proj`, and whether REAPER makes that project
+                            // current for the action's duration is `[verify — DAW]`
    int unreadable = 0;     // IgnoreUnreadable
    int notARequest = 0;    // IgnoreNotARequest
    int staleCleared = 0;   // ClearStale
-    int answered = 0;       // an outcome was written back (a landing OR a refusal)
+    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 landed = 0;         // of `answered`, the ones that reached the bank
 };

-// The console sentence for a pass that answered NOTHING — the one state in which the
-// asking instance reports a no-answer and has nothing to go on. Empty string when
-// `answered > 0`, so the caller prints unconditionally and stays quiet on a pass that
-// spoke for itself. The line's absence is itself evidence: no line means the landing
-// action never ran.
+// 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.
 std::string describeBakeScan(const BakeScanTally& tally);

+// 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
+// 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
+};
+
+// 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.
+std::string describeBakeKey(const std::string& key, const BakeKeyOutcome& outcome);
+
 }  // namespace reasampler::wire