Narrow the FX reclaimer's transport gate to defer offline work only; let online drain through, and always run the park clock's observe

src/shell/view/fx_reclaim.cpp

@@ -110,8 +110,8 @@ struct ReclaimHost {
     }
 
     // The SAME predicate applyMode refuses a real mode switch on, deliberately shared
-    // rather than re-read here: what counts as a transport that must not have plugins
+    // rather than re-read here: "the transport is rolling" is one fact with one home
-    // moved under it is one fact with one home (view.h).
+    // (view.h). drainTick narrows what it gates with this answer to offline work only.
     bool transportRolling() const { return transportBlocksModeSwitch(project); }
 
     bool liveChain(const std::string& guid, std::vector<std::string>& out) {

src/shell/view/fx_reclaim.h

@@ -173,8 +173,9 @@ struct ReclaimTickResult {
 // fx_reclaim.cpp holds the one live implementation. Host must provide:
 //
 //   double nowMs()                                       monotonic milliseconds
-//   bool   transportRolling()                            playing or recording — the tick
+//   bool   transportRolling()                            playing or recording — gates the
-//                                                        may move no plugins at all
+//                                                        OFFLINE direction only; a tick
+//                                                        may still online while it rolls
 //   bool   liveChain(guid, std::vector<std::string>&)    FX identity per live slot;
 //                                                        false when the track is gone
 //   bool   writeOffline(guid, fxIndex, offline)          compare-before-write inside its
@@ -194,20 +195,30 @@ template <class Host>
 ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, Host& host) {
     ReclaimTickResult result;
 
-    // Mirrors applyMode's refusal (view.h, transportBlocksModeSwitch), and now covers
+    // Run every tick, rolling or not: matured()'s own contract ("observe() runs first
-    // every FX write the tool makes, since this loop owns them all. BOTH directions, not
+    // every tick") already assumed this, and a track can stop owing offline work on a
-    // just the opportunistic offline one: onlining re-instantiates the very plugins the
+    // rolling tick too now that this loop keeps writing online through one — a
-    // user is listening through — the same destruction the switch already refuses — and a
+    // continuously-parked track's origin is untouched (observe carries the prior
-    // step is a whole chain with no yield inside it. Nothing is observed on this path, so
+    // timestamp forward), so the only thing this changes is dropping a hold the moment
-    // every debounce hold keeps its origin and the first ungated tick fires with no wait.
+    // its track genuinely stops owing offline work, matured or not.
-    if (host.transportRolling()) {
-        result.settled = queueSettled(items);
-        return result;
-    }
-
     clock.observe(items, host.nowMs());
 
-    const int first = nextEligible(items, clock, host.nowMs());
+    // Mirrors applyMode's refusal (view.h, transportBlocksModeSwitch), narrowed to the
+    // OFFLINE direction only: onlining is the mode switch's own unfinished promise — the
+    // user is waiting on it — while offlining is opportunistic reclaim that can always
+    // wait for the transport to stop. Onlining while rolling still costs a plugin load
+    // mid-playback; that is an accepted trade for this gate's shape, not a hazard it
+    // introduces.
+    const bool offlineAllowed = !host.transportRolling();
+    auto nextAllowed = [&](double nowMs) {
+        const int i = nextEligible(items, clock, nowMs);
+        if (i >= 0 && !offlineAllowed &&
+            items[static_cast<std::size_t>(i)].work.kind == view::FxWorkKind::Offline)
+            return -1;
+        return i;
+    };
+
+    const int first = nextAllowed(host.nowMs());
     if (first < 0) {
         result.settled = queueSettled(items);
         return result;
@@ -218,7 +229,7 @@ ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, H
     const double start  = host.nowMs();
 
     for (;;) {
-        const int i = nextEligible(items, clock, host.nowMs());
+        const int i = nextAllowed(host.nowMs());
         if (i < 0) break;
         ReclaimItem& item = items[static_cast<std::size_t>(i)];
         // Offline work never rides the online budget: the two directions are priced

tests/test_fx_reclaim.cpp

@@ -869,7 +869,7 @@ static void testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo() {
 
 // -- 9. the transport gate ------------------------------------------------------------
 
-static void testARollingTransportMovesNoPluginsAndKeepsTheHold() {
+static void testARollingTransportDefersOfflineWorkAndKeepsTheHold() {
     ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
     FakeHost host;
     host.chains["{T}"] = onlineChain({"{FX-A}", "{FX-B}"});
@@ -883,7 +883,7 @@ static void testARollingTransportMovesNoPluginsAndKeepsTheHold() {
     CHECK(gated.writes == 0);
     CHECK(host.writeLog.empty());
     CHECK(!gated.aborted);
-    CHECK(!gated.settled);  // the work is still owed, so the shell must not go idle
+    CHECK(!gated.settled);  // the work is still owed — a gated item is deferred, not dropped
     CHECK(!q.stale());      // and the queue it will resume from survives untouched
 
     // The hold kept its origin across the gated tick: the drain fires on the FIRST tick
@@ -893,9 +893,12 @@ static void testARollingTransportMovesNoPluginsAndKeepsTheHold() {
     CHECK(host.writeLog.size() == 2);
 }
 
-static void testTheGateStopsTheOnlineDirectionToo() {
+static void testOnlineWorkProceedsWhileTheTransportRolls() {
-    // The ruling is symmetric because both directions load or unload plugins: onlining
+    // Daniel's ruling: the gate narrows to the offline direction. Onlining is the mode
-    // re-instantiates the very chain the user is listening through.
+    // switch's own unfinished promise — restoreTrack already handed back visibility and
+    // B_MAINSEND/I_FXEN synchronously — so a full gate would leave the user listening to a
+    // dry, FX-enabled chain for the whole rest of the playback pass. Onlining still costs
+    // a plugin load mid-roll; that is the accepted trade, not a regression this test flags.
     ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
     vm.setActiveMode(kDesignModeId);
     vm.releaseSnapshotFlags("{T}");
@@ -909,19 +912,87 @@ static void testTheGateStopsTheOnlineDirectionToo() {
 
     FxReclaimQueue q = queueOf(vm, {"{T}"});
     CHECK(q.items().size() == 1 && q.items()[0].work.kind == FxWorkKind::Online);
-    const ReclaimRunResult gated = q.run(host);
+    const ReclaimRunResult run = q.run(host);
-    CHECK(gated.writes == 0);
+    CHECK(run.writes == 2);
-    CHECK(!gated.settled);
+    CHECK(run.settled);
-    CHECK(host.released.empty());       // no record spent while the transport rolls
-    CHECK(!flush.pendingFor(kProjA));
-    CHECK(host.offlineAt("{T}", 0));    // the chain is untouched, still parked
-
-    // The contrast, so the assertions above are about the gate and not about an online
-    // queue that cannot drain: stop the transport and the same tick completes.
-    host.rolling = false;
-    CHECK(q.run(host).settled);
     CHECK(host.writeLog.size() == 2);
-    CHECK(host.released.count("{T}") == 1);
+    CHECK(host.released.count("{T}") == 1);   // the record's FX half is spent
+    CHECK(flush.pendingFor(kProjA));
+    CHECK(!host.offlineAt("{T}", 0) && !host.offlineAt("{T}", 1));
+}
+
+static void testOfflineWorkStaysGatedWhileOnlineDrainsInTheSameTick() {
+    // A mixed queue while rolling: {Z-back} is owed an online, {A-park}'s hold is already
+    // matured and owed an offline. Only the online direction may move this tick.
+    ViewModeModel vm;
+    vm.membership().tag("{Z-back}", kArrangeModeId);
+    vm.membership().tag("{A-park}", kDesignModeId);
+    vm.storeSnapshot("{Z-back}", recordWithFx({{"{FX-1}", 0}}));
+    vm.storeSnapshot("{A-park}", recordWithFx({{"{FX-2}", 0}}));
+
+    FakeHost host;
+    host.chains["{Z-back}"] = offlineChain({"{FX-1}"});
+    host.chains["{A-park}"] = onlineChain({"{FX-2}"});
+    ParkClock clock;
+
+    std::vector<ReclaimItem> items = derive(vm, {"{Z-back}", "{A-park}"});
+    CHECK(items[0].work.kind == FxWorkKind::Online);
+    matureHold(items, clock, host);  // {A-park}'s hold matures with time to spare below
+
+    host.rolling = true;
+    host.clock += kParkHoldMs;
+    const ReclaimTickResult tick = drainTick(items, clock, host);
+    CHECK(tick.writes == 1);
+    CHECK(host.writeLog == std::vector<std::string>{"{Z-back}:0=on"});
+    CHECK(!host.offlineAt("{A-park}", 0));  // matured but gated — untouched this tick
+    CHECK(!tick.settled);                   // the offline item is still owed
+
+    // The transport stops: the very next tick reclaims it, with no fresh hold to wait out.
+    host.rolling = false;
+    CHECK(drainTick(items, clock, host).settled);
+    CHECK(host.offlineAt("{A-park}", 0));
+}
+
+static void testAHoldDropsWhenItsTrackStopsOwingOfflineWorkWhileRolling() {
+    // ParkClock's own contract (fx_reclaim.h) says an entry is dropped the moment its
+    // track stops owing offline work — observe() has to run every tick, rolling or not,
+    // for that to hold once a rolling tick can itself change what a track owes (by
+    // draining its online item). This is the case that contract would otherwise miss.
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
+    FakeHost host;
+    // Offline in the DAW, matching a track truly parked — so bringing it online below is a
+    // real write, not a compare-before-write no-op.
+    host.chains["{T}"] = offlineChain({"{FX-A}"});
+    ParkClock clock;
+
+    std::vector<ReclaimItem> items = derive(vm, {"{T}"});
+    CHECK(drainTick(items, clock, host).writes == 0);  // the hold starts at t=0
+    CHECK(!clock.matured("{T}", 0.0));
+
+    // {T} comes back online while the transport rolls; the online direction still drains.
+    host.rolling = true;
+    vm.setActiveMode(kDesignModeId);
+    vm.releaseSnapshotFlags("{T}");
+    items = derive(vm, {"{T}"});
+    CHECK(items[0].work.kind == FxWorkKind::Online);
+    host.clock = 100.0;
+    CHECK(drainTick(items, clock, host).writes == 1);
+    CHECK(!clock.matured("{T}", host.clock));  // dropped: {T} owes no offline work now
+
+    // {T} parks again, still rolling: the hold must start FRESH from here, not resume the
+    // one interrupted above — proof the drop above was real, not a no-op.
+    vm.setActiveMode(kArrangeModeId);
+    vm.storeSnapshot("{T}", recordWithFx({{"{FX-A}", 0}}));
+    items = derive(vm, {"{T}"});
+    CHECK(items[0].work.kind == FxWorkKind::Offline);
+    const double repark = host.clock;
+    CHECK(!drainTick(items, clock, host).settled);  // gated, and not matured either
+    CHECK(!clock.matured("{T}", repark + kParkHoldMs - 1.0));
+    CHECK(clock.matured("{T}", repark + kParkHoldMs));
+
+    host.rolling = false;
+    host.clock = repark + kParkHoldMs;
+    CHECK(drainTick(items, clock, host).writes == 1);  // matures once the gate lifts
 }
 
 static void testAGatedTickStillGoesIdleWithNothingLeftToConverge() {
@@ -971,8 +1042,10 @@ int main() {
     testOnlyANoProjectTickClearsTheWakeup();
     testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo();
 
-    testARollingTransportMovesNoPluginsAndKeepsTheHold();
+    testARollingTransportDefersOfflineWorkAndKeepsTheHold();
-    testTheGateStopsTheOnlineDirectionToo();
+    testOnlineWorkProceedsWhileTheTransportRolls();
+    testOfflineWorkStaysGatedWhileOnlineDrainsInTheSameTick();
+    testAHoldDropsWhenItsTrackStopsOwingOfflineWorkWhileRolling();
     testAGatedTickStillGoesIdleWithNothingLeftToConverge();
 
     if (g_fail == 0) std::printf("All tests passed.\n");