Fix five Wave-1 pure-layer FX/park review findings: resume-point stall, stale doc comments, implicit mode read, keying reset

src/core/view/applied_state.h

@@ -1,9 +1,13 @@
 #pragma once
 // A comparable fingerprint of what an apply put into the project, so a reconcile
-// can ask "does the model differ from what we applied?" rather than "did an undo
-// happen?". The second question is not the same one: an undo that touched
-// nothing we drive still answers yes, and repairing on it re-enables state the
-// user disabled by hand.
+// can ask "did our applied output drift from what a restore would drive it back
+// to?" rather than "did an undo happen?". The second question is not the same
+// one: an undo that touched nothing we drive still answers yes, and repairing on
+// it re-enables state the user disabled by hand. Narrower still than "did the
+// model change?": membership, lane ownership and the solo cache are excluded by
+// design (see AppliedState below), so an edit to any of those changes what a
+// future apply must drive while leaving this fingerprint bit-identical. Not a
+// sole reapply trigger for anything membership can affect.
 
 #include <string>
 #include <vector>

src/core/view/fx_state.cpp

@@ -7,15 +7,22 @@ namespace reasampler::view {
 
 namespace {
 
-// Everything up to and including the first op naming `resumeAfter` is already
-// done. Not found ⇒ nothing dropped: the identity left the chain between ticks,
-// and re-walking a compare-before-write list beats trusting a cursor that no
-// longer points at anything.
+// Everything up to and including the resume point is already done. An empty
+// `resumeAfterFxGuid` is ambiguous by itself — "nothing done yet" and "the last
+// completed op had no identity" look identical — so an empty guid falls back to
+// matching by slot instead; `resumeAfterSlot < 0` is what actually means
+// "nothing done yet". Not found ⇒ nothing dropped: the resume point left the
+// chain between ticks, and re-walking a compare-before-write list beats
+// trusting a cursor that no longer points at anything.
 std::vector<FxOfflineOp> remainingAfter(const std::vector<FxOfflineOp>& ops,
-                                        const std::string& resumeAfter) {
-    if (resumeAfter.empty()) return ops;
+                                        const std::string& resumeAfterFxGuid,
+                                        int resumeAfterSlot) {
+    if (resumeAfterFxGuid.empty() && resumeAfterSlot < 0) return ops;
     for (std::size_t i = 0; i < ops.size(); ++i) {
-        if (ops[i].fxGuid == resumeAfter)
+        const bool isResumePoint = resumeAfterFxGuid.empty()
+                                        ? ops[i].slot == resumeAfterSlot
+                                        : ops[i].fxGuid == resumeAfterFxGuid;
+        if (isResumePoint)
             return std::vector<FxOfflineOp>(ops.begin() + static_cast<std::ptrdiff_t>(i) + 1,
                                             ops.end());
     }
@@ -54,15 +61,15 @@ std::vector<FxOfflineOp> fxRestoreTargets(const ViewModeModel& model,
 }
 
 std::vector<FxTrackWork> desiredFxState(const ViewModeModel& model,
+                                        const std::string& targetMode,
                                         const std::set<std::string>& liveTrackGuids) {
     std::vector<FxTrackWork> online;
     std::vector<FxTrackWork> offline;
 
-    const std::string& activeMode = model.activeModeId();
     for (const auto& [guid, snap] : model.snapshots()) {  // std::map ⇒ GUID order
         if (liveTrackGuids.count(guid) == 0) continue;  // dead track — nothing to write
 
-        if (!model.leafBelongsToMode(guid, activeMode)) {
+        if (!model.leafBelongsToMode(guid, targetMode)) {
             offline.push_back(FxTrackWork{guid, FxWorkKind::Offline, {}, {}});
             continue;
         }
@@ -82,10 +89,12 @@ FxRestoreResolution resolveFxWork(const FxTrackWork& work,
                                   const std::vector<std::string>& liveFxGuids) {
     if (work.kind == FxWorkKind::Offline) {
         return resolveFxRestore(
-            remainingAfter(offlineTargets(work.trackGuid, liveFxGuids), work.resumeAfterFxGuid),
+            remainingAfter(offlineTargets(work.trackGuid, liveFxGuids), work.resumeAfterFxGuid,
+                          work.resumeAfterSlot),
             liveFxGuids);
     }
-    return resolveFxRestore(remainingAfter(work.targets, work.resumeAfterFxGuid), liveFxGuids);
+    return resolveFxRestore(
+        remainingAfter(work.targets, work.resumeAfterFxGuid, work.resumeAfterSlot), liveFxGuids);
 }
 
 }  // namespace reasampler::view

src/core/view/fx_state.h

@@ -32,14 +32,28 @@ struct FxTrackWork {
     // read it, so resolveFxWork synthesises the targets from what it is handed.
     std::vector<FxOfflineOp> targets;
 
-    // Identity of the last FX this item's drain completed; empty ⇒ none yet. An
-    // optimisation only: every write is compare-before-write, so an identity that
-    // has since left the chain safely restarts the item rather than stranding it.
+    // Identity of the last FX this item's drain completed; empty means EITHER
+    // nothing has completed yet OR the completed FX itself carried no identity
+    // (REAPER reports no GUID for it, or it came from a Slot-keyed record) — see
+    // resumeAfterSlot below, which disambiguates the two. This is load-bearing
+    // for forward progress, not an optimisation: without the disambiguation, an
+    // unidentified FX would look like "none yet" and the drain would restart at
+    // the top of the list every tick, forever. Compare-before-write makes every
+    // write idempotent, so an identity that has since left the chain safely
+    // restarts the item rather than stranding it.
     std::string resumeAfterFxGuid;
 
+    // Position of the last completed op, consulted only when resumeAfterFxGuid
+    // is empty, to tell "resumed past an unidentified FX at this slot" apart
+    // from "nothing done yet" (-1). Every FxOfflineOp carries its slot
+    // regardless of keying, so this is available for both Online and Offline
+    // work.
+    int resumeAfterSlot = -1;
+
     bool operator==(const FxTrackWork& o) const {
         return trackGuid == o.trackGuid && kind == o.kind && targets == o.targets &&
-               resumeAfterFxGuid == o.resumeAfterFxGuid;
+               resumeAfterFxGuid == o.resumeAfterFxGuid &&
+               resumeAfterSlot == o.resumeAfterSlot;
     }
 };
 
@@ -51,7 +65,13 @@ struct FxTrackWork {
 // opportunistic reclaim that may be deferred indefinitely without losing
 // anything. Within a kind, GUID order — deterministic, so a drain's position in
 // the list survives a re-derivation.
+//
+// `targetMode` is explicit, mirroring planToggle, rather than read from
+// `model.activeModeId()`: applyMode sets the active mode LAST, after its park
+// and restore loops, so a caller reading it from inside or before that block
+// would classify every track against the OUTGOING mode instead.
 std::vector<FxTrackWork> desiredFxState(const ViewModeModel& model,
+                                        const std::string& targetMode,
                                         const std::set<std::string>& liveTrackGuids);
 
 // One track's Online targets from its park record's FX half; empty when the

src/core/view/view_mode_model.cpp

@@ -278,7 +278,9 @@ void ViewModeModel::releaseSnapshotFx(const std::string& guid) {
     if (it == snapshots_.end()) return;
     it->second.fxHeld = false;
     it->second.fxOffline.clear();
-    it->second.fxKeying = FxKeying::Identity;
+    // fxKeying is left as captured: the record's own keying, not a claim about
+    // one it never had. Harmless today (fxOffline is empty right above), but
+    // whoever reads fxKeying after this point should see the truth.
     eraseIfSpent(snapshots_, it);
 }
 

tests/test_applied_state.cpp

@@ -102,10 +102,15 @@ static void testEditsTheApplyDidNotDriveAreNotDifferences() {
     ViewModeModel vm = parkedPair();
     const AppliedState before = appliedState(vm);
 
-    vm.membership().tag("{OTHER}", kDesignModeId);
+    // {T1} is already parked (tagged Design, active mode Arrange); retagging it
+    // INTO the active mode changes what the next apply must drive (release it)
+    // while its park record — and so the fingerprint — is untouched. Pinning
+    // this on a GUID actually in the parked set is the point: an unrelated GUID
+    // would pass even if membership were folded into the fingerprint by mistake.
+    vm.membership().tag("{T1}", kArrangeModeId);
     CHECK(appliedState(vm) == before);
 
-    vm.membership().setShowBoth("{OTHER}", true);
+    vm.membership().setShowBoth("{T1}", true);
     CHECK(appliedState(vm) == before);
 
     vm.lanes().setManaged("{T1}", "reasampler:design", kDesignModeId);

tests/test_fx_state.cpp

@@ -63,7 +63,7 @@ static void testActiveTrackIsOwedOnlineInactiveIsOwedOffline() {
     vm.storeSnapshot("{P}", recordWithFx({{"{FX-P}", 0}}));
 
     // Active mode is Arrange: {T} (Design) stays parked, {P} is coming back.
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}", "{P}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kArrangeModeId, {"{T}", "{P}"});
     CHECK(work.size() == 2);
     CHECK(workFor(work, "{P}") && workFor(work, "{P}")->kind == FxWorkKind::Online);
     CHECK(workFor(work, "{T}") && workFor(work, "{T}")->kind == FxWorkKind::Offline);
@@ -86,7 +86,7 @@ static void testOnlineWorkSortsBeforeOfflineWork() {
         vm.storeSnapshot(g, recordWithFx({{"{FX}", 0}}));
 
     const std::vector<FxTrackWork> work =
-        desiredFxState(vm, {"{A-park}", "{B-park}", "{Y-back}", "{Z-back}"});
+        desiredFxState(vm, kArrangeModeId, {"{A-park}", "{B-park}", "{Y-back}", "{Z-back}"});
     CHECK(work.size() == 4);
     // Onlining restores state the user owns and is 2-6x the cost of offlining;
     // offlining is opportunistic reclaim and may be deferred indefinitely.
@@ -97,7 +97,8 @@ static void testOnlineWorkSortsBeforeOfflineWork() {
 
     // Deterministic: a re-derivation yields the identical list, so a drain's
     // position in it survives one.
-    CHECK(desiredFxState(vm, {"{A-park}", "{B-park}", "{Y-back}", "{Z-back}"}) == work);
+    CHECK(desiredFxState(vm, kArrangeModeId, {"{A-park}", "{B-park}", "{Y-back}", "{Z-back}"}) ==
+          work);
 }
 
 static void testUserOfflinedPluginIsNeverBlanketOnlined() {
@@ -106,7 +107,7 @@ static void testUserOfflinedPluginIsNeverBlanketOnlined() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
 
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     CHECK(work[0].kind == FxWorkKind::Online);
 
@@ -124,14 +125,14 @@ static void testReleasedFxHalfIsOwedNothing() {
     CHECK(vm.setActiveMode(kDesignModeId));
     vm.releaseSnapshotFx("{T}");
 
-    CHECK(desiredFxState(vm, {"{T}"}).empty());
+    CHECK(desiredFxState(vm, kDesignModeId, {"{T}"}).empty());
     CHECK(fxRestoreTargets(vm, "{T}").empty());
 
     // ...and the mirror: a record whose FLAG half is spent still owes its FX.
     ViewModeModel vm2 = parkedWithThreeFx();
     CHECK(vm2.setActiveMode(kDesignModeId));
     vm2.releaseSnapshotFlags("{T}");
-    const std::vector<FxTrackWork> work = desiredFxState(vm2, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm2, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     CHECK(work.size() == 1 && work[0].targets.size() == 3);
 }
@@ -139,20 +140,20 @@ static void testReleasedFxHalfIsOwedNothing() {
 static void testDeadAndUnparkedTracksAreOwedNothing() {
     ViewModeModel vm = parkedWithThreeFx();
     // The track is gone from the project: there is nothing to write to.
-    CHECK(desiredFxState(vm, {}).empty());
+    CHECK(desiredFxState(vm, kArrangeModeId, {}).empty());
 
     // A track with no park record is owed nothing either way — that is also how a
     // REFUSED park stays refused: it wrote no record, so no reclaim targets it.
     ViewModeModel bare;
     bare.membership().tag("{U}", kDesignModeId);
-    CHECK(desiredFxState(bare, {"{U}"}).empty());
+    CHECK(desiredFxState(bare, kArrangeModeId, {"{U}"}).empty());
 }
 
 // -- resolveFxWork: against the chain as it stands ----------------------------
 
 static void testOfflineWorkTargetsEveryLiveSlot() {
     ViewModeModel vm = parkedWithThreeFx();  // active mode Arrange ⇒ {T} parks
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kArrangeModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -168,7 +169,7 @@ static void testOfflineWorkTargetsEveryLiveSlot() {
 static void testReorderRestoreLandsEachPluginItsOwnState() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -184,7 +185,7 @@ static void testReorderRestoreLandsEachPluginItsOwnState() {
 static void testDeletedFxIsDroppedExplicitly() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -200,7 +201,7 @@ static void testDeletedFxIsDroppedExplicitly() {
 static void testAddedFxIsLeftAlone() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -228,7 +229,7 @@ static void testSlotKeyedRecordStillRestoresByPosition() {
     CHECK(targets.size() == 3);
     CHECK(!targets.empty() && targets[0].keying == FxKeying::Slot);
 
-    const std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    const std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
     const FxRestoreResolution res = resolveFxWork(work[0], {"{FX-X}", "{FX-Y}", "{FX-Z}"});
@@ -244,7 +245,7 @@ static void testSlotKeyedRecordStillRestoresByPosition() {
 static void testResumePointSkipsWhatTheDrainAlreadyDid() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
-    std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -263,7 +264,7 @@ static void testResumePointSkipsWhatTheDrainAlreadyDid() {
 static void testResumePointThatLeftTheChainRestartsRatherThanStrands() {
     ViewModeModel vm = parkedWithThreeFx();
     CHECK(vm.setActiveMode(kDesignModeId));
-    std::vector<FxTrackWork> work = desiredFxState(vm, {"{T}"});
+    std::vector<FxTrackWork> work = desiredFxState(vm, kDesignModeId, {"{T}"});
     CHECK(work.size() == 1);
     if (work.empty()) return;
 
@@ -277,7 +278,7 @@ static void testResumePointThatLeftTheChainRestartsRatherThanStrands() {
 
     // Same rule on the offline side, where the cursor keys into the live chain.
     ViewModeModel parked = parkedWithThreeFx();  // Arrange active ⇒ {T} parks
-    std::vector<FxTrackWork> offlineWork = desiredFxState(parked, {"{T}"});
+    std::vector<FxTrackWork> offlineWork = desiredFxState(parked, kArrangeModeId, {"{T}"});
     CHECK(offlineWork.size() == 1);
     if (offlineWork.empty()) return;
     offlineWork[0].resumeAfterFxGuid = "{FX-A}";
@@ -286,6 +287,27 @@ static void testResumePointThatLeftTheChainRestartsRatherThanStrands() {
     CHECK(resolveFxWork(offlineWork[0], {"{FX-A}", "{FX-B}", "{FX-C}"}).writes.size() == 3);
 }
 
+static void testResumeAfterUnidentifiedFxMakesForwardProgress() {
+    // The drain completed slot 1, an FX REAPER reports no GUID for. Before the
+    // slot-cursor fix, an empty resumeAfterFxGuid was indistinguishable from
+    // "nothing done yet" and this item would restart at slot 0 every tick,
+    // forever — the non-terminating drain this test pins against.
+    ViewModeModel parked = parkedWithThreeFx();  // Arrange active ⇒ {T} parks (Offline, slot-keyed)
+    std::vector<FxTrackWork> work = desiredFxState(parked, kArrangeModeId, {"{T}"});
+    CHECK(work.size() == 1);
+    if (work.empty()) return;
+
+    work[0].resumeAfterFxGuid = "";
+    work[0].resumeAfterSlot   = 1;
+    const FxRestoreResolution res =
+        resolveFxWork(work[0], {"{FX-A}", "", "{FX-B}", "{FX-C}"});
+    CHECK(res.writes.size() == 2);   // slots 2 and 3 only — 0 and 1 already done
+    CHECK(!anyWriteAt(res, 0));
+    CHECK(!anyWriteAt(res, 1));
+    CHECK(anyWriteAt(res, 2));
+    CHECK(anyWriteAt(res, 3));
+}
+
 int main() {
     testActiveTrackIsOwedOnlineInactiveIsOwedOffline();
     testOnlineWorkSortsBeforeOfflineWork();
@@ -301,6 +323,7 @@ int main() {
 
     testResumePointSkipsWhatTheDrainAlreadyDid();
     testResumePointThatLeftTheChainRestartsRatherThanStrands();
+    testResumeAfterUnidentifiedFxMakesForwardProgress();
 
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;