diff --git a/src/shell/view/CLAUDE.md b/src/shell/view/CLAUDE.md index 5135ef8..08b40e4 100644 --- a/src/shell/view/CLAUDE.md +++ b/src/shell/view/CLAUDE.md @@ -113,6 +113,22 @@ decide membership or mode rules. `abandon`. Daniel's ruling, and the accepted cost: undoing a switch mid-drain leaves a PARTIAL park — some FX offlined, some not — covered by the one point the terminator closed. +- **A state load puts the reclaimer to SLEEP; only a re-read of the model wakes it.** + `abandon` clears the wakeup where `invalidate` raises it, because the two say + different things: an invalidation says the MODEL moved and owes a re-derivation, a + load says the PROJECT moved under a model nobody has re-read yet. `applyMode` is + `invalidateFxReclaim`'s one caller and `fxReclaimPending` gates the tick that is the + one path to any FX write or park-record spend, so every load that re-reads the model + wakes the loop on the same tick, through the reapply the reload drives — and a load + that re-reads nothing wakes nothing. `File > Revert` is exactly that load: same + `ReaProject`, same GUID, same path, so the identity poll answers `NoOp` and the model + is never re-read. Waking there was the only thing that ran, and it converged the FX + chains toward the pre-revert model and spent the park records the reverted project + still needed. The accepted cost of the sleep: an outstanding reclaim in a project + whose interrupting load was some OTHER project's waits for that project's next apply + rather than resuming on the next tick. (That a revert leaves the view model stale at + all is the identity poll's defect, not the reclaimer's — this invariant only keeps the + reclaimer from acting on it.) - **A reapply that wrote nothing leaves no undo point** — and a reapply never holds the span open, so a project open, a membership edit and `render_in_place`'s inner reapply all keep the block shape they had. The load-tick reapply diff --git a/src/shell/view/fx_reclaim.cpp b/src/shell/view/fx_reclaim.cpp index 7d1c4ce..abe8077 100644 --- a/src/shell/view/fx_reclaim.cpp +++ b/src/shell/view/fx_reclaim.cpp @@ -281,9 +281,9 @@ void fxReclaimLabelUndoSpan(const char* label, int mask) { void fxReclaimEndUndoSpan() { closeSpan(); } void fxReclaimAbandonForStateLoad() { - // Close FIRST and unconditionally. The abandon below only stops future writes; the - // block is what a Ctrl-Z would otherwise close over the wrong state, and it is open - // whether or not a drain is on the stack at all. + // Close FIRST and unconditionally. The abandon below stops the drain and puts the loop + // to sleep; the block is what a Ctrl-Z would otherwise close over the wrong state, and + // it is open whether or not a drain is on the stack at all. closeSpan(); g_queue.abandon(); } diff --git a/src/shell/view/fx_reclaim.h b/src/shell/view/fx_reclaim.h index 7162df7..71f2e7e 100644 --- a/src/shell/view/fx_reclaim.h +++ b/src/shell/view/fx_reclaim.h @@ -369,9 +369,25 @@ public: // closed. A load that keeps the SAME ReaProject (a revert) is what makes the stop // load-bearing rather than tidy: the drain's pin and cached track handle both still // validate afterwards, so nothing else would keep it out of the reloaded project. + // + // The wakeup goes DOWN, where an invalidation raises it — do not reintroduce the raise. + // An invalidation says the MODEL moved and owes a re-derivation; a load says the + // PROJECT moved under a model nobody has re-read yet, which is the opposite of a reason + // to converge. Every load that re-reads the model raises the wakeup itself, on that + // same tick, through the reapply the reload drives (applyMode is invalidateFxReclaim's + // one caller); File > Revert re-reads nothing, so waking here was the only thing that + // ever ran, and it wrote FX and spent park records against the pre-revert model. + // + // Safe to clear state under a drain, unlike invalidate(): the queue's own storage is + // empty for the drain's whole duration (run() moved it out), and run()'s abort path + // returns before it would read derived_ or commit anything. void abandon() { - invalidate(); + ++generation_; abandoned_ = true; + discard(); + pending_ = false; + // The pin STAYS — it is what the next tick's retarget tests the debounce clock + // against, and that clock outlives this exactly as it outlives an invalidation. } // Read by the shell's host on every step; false again from the next run(), because the @@ -666,8 +682,8 @@ void fxReclaimLabelUndoSpan(const char* label, int mask); void fxReclaimEndUndoSpan(); // EVERY project-state load, undo-driven or not — Ctrl-Z, Ctrl-Shift-Z, a project open, a -// File > Revert, a new project. Ends the span and stops any drain on the stack -// (FxReclaimQueue::abandon). +// File > Revert, a new project. Ends the span, stops any drain on the stack, and leaves the +// loop ASLEEP until something that re-read the model wakes it (FxReclaimQueue::abandon). // // Called from the projectconfig BeginLoadProjectState hook, which fires BEFORE REAPER // restores anything: that is the ONLY point at which the block can still close over the diff --git a/tests/test_fx_reclaim.cpp b/tests/test_fx_reclaim.cpp index 6987ae1..dfb2529 100644 --- a/tests/test_fx_reclaim.cpp +++ b/tests/test_fx_reclaim.cpp @@ -222,6 +222,18 @@ FxReclaimQueue queueOf(const ViewModeModel& vm, const std::set& liv return q; } +// One whole timer tick as the shell makes it: main.cpp's ONE pending() test, then +// fxReclaimTick's pin, its re-derivation from the session's model whenever the queue is +// stale, and the drain. The re-derivation is what makes the wakeup the load-bearing bit — +// it reads whatever model is live right now, whether or not anything re-read it since. +void timerTick(FxReclaimQueue& q, const ViewModeModel& vm, const std::set& live, + FakeHost& host) { + if (!q.pending()) return; + if (!q.beginTick(host.project)) return; + if (q.stale()) q.adopt(derive(vm, live)); + q.run(host); +} + } // namespace // -- 1. debounce --------------------------------------------------------------- @@ -1254,10 +1266,11 @@ static void testAStateLoadStopsTheDrainWhereItStandsRatherThanFinishingTheTrack( CHECK(spanEndsAfter(r)); // the tick's own close still fires... CHECK(!span.end()); // ...and finds nothing: the terminator got there first - // The wakeup stands and the work is owed again: going idle here would strand the - // half-parked chain for the rest of the session. + // The work is owed again, and the loop is ASLEEP until something re-reads the model — + // the reapply a reload drives is what raises the wakeup, and a load that drives none + // must raise nothing (testARevertBetweenTicksLeavesTheLoopAsleepOnAStaleModel). CHECK(q.stale()); - CHECK(q.pending()); + CHECK(!q.pending()); // ...and the stop belonged to that tick alone. The reloaded project's own drain writes. host.abandonAfterWrite = -1; @@ -1267,6 +1280,57 @@ static void testAStateLoadStopsTheDrainWhereItStandsRatherThanFinishingTheTrack( CHECK(host.offlineAt("{T2}", 0)); } +static void testARevertBetweenTicksLeavesTheLoopAsleepOnAStaleModel() { + // File > Revert is the load that re-reads NOTHING: it keeps the same ReaProject, so the + // session's identity poll answers NoOp, no reload follows and therefore no reapply + // does either. The terminator is the only thing that runs, so whether it raises the + // wakeup decides the whole outcome. Online work is the sharp case — undebounced, so ONE + // woken tick both writes FX and spends the record's FX half, against the very model the + // user reverted away from. The contrast with the mid-drain terminator test above: there + // the load arrives through a write's own message pump, here between two ticks entirely. + ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"}); + vm.setActiveMode(kDesignModeId); + vm.releaseSnapshotFlags("{T}"); + + FakeHost host; + host.model = &vm; + host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"}); + ReleaseFlush flush; + host.flush = &flush; + + FxReclaimQueue q; + CHECK(q.beginTick(kProjA)); // where an earlier tick left it: pinned, nothing owed + q.invalidate(); // applyMode, leaving the switch that brought {T} back + CHECK(q.pending()); + + q.abandon(); + CHECK(!q.pending()); + CHECK(q.stale()); + CHECK(q.items().empty()); + CHECK(q.project() == kProjA); // the pin stays: it is what guards the debounce clock + + // Timer ticks keep arriving. The queue is empty AND stale, so it is the wakeup alone + // that keeps the drain out — timerTick re-derives from the stale model exactly as the + // shell would, which is what a raise here would have handed the drain. + for (int i = 0; i < 3; ++i) timerTick(q, vm, {"{T}"}, host); + CHECK(host.writeLog.empty()); + CHECK(host.released.empty()); + CHECK(!flush.pendingFor(kProjA)); + // ...and the chain is untouched — still parked offline, as the revert left it. + CHECK(host.offlineAt("{T}", 0) && host.offlineAt("{T}", 1)); + + // The contrast, so the assertions above are about the wakeup and not about this queue + // being unable to converge: a load that DOES re-read the model reaches the reclaimer + // through its reapply's invalidate, and the very same tick over the very same model + // both writes and spends. + q.invalidate(); + timerTick(q, vm, {"{T}"}, host); + CHECK(host.writeLog.size() == 2); + CHECK(!host.offlineAt("{T}", 0) && !host.offlineAt("{T}", 1)); + CHECK(host.released.count("{T}") == 1); + CHECK(flush.pendingFor(kProjA)); +} + // -- 12. the direction the drain is moving ---------------------------------------------- static void testTheDirectionReadsOnlineWhileTheParksOwnOfflineWorkWaitsOutItsHold() { @@ -1413,6 +1477,7 @@ int main() { testADrainNoSwitchOpenedMintsOnePointOfItsOwn(); testAStateLoadStopsTheDrainWhereItStandsRatherThanFinishingTheTrack(); + testARevertBetweenTicksLeavesTheLoopAsleepOnAStaleModel(); testTheDirectionReadsOnlineWhileTheParksOwnOfflineWorkWaitsOutItsHold(); testAnOfflineOnlyQueueReadsIdleUntilItsHoldMatures();