Collapse a Design View mode switch to ONE undo point: FX writes run inline in applyMode's block, deferred park queue deleted

src/shell/view/CLAUDE.md

@@ -37,15 +37,15 @@ decide membership or mode rules.
   The FX half asks "is ANY FX offline", not "every" — no read distinguishes a
   park's leftover from one the user set by hand, and only "any" errs toward
   committing nothing false. An absent snapshot is NOT evidence of a clean chain —
-  `discardDeferredFxParks` drops intents whose flag writes already landed, so a
-  reloaded model and an already-parked project routinely coexist. A park meeting
-  that pair REFUSES: no flag write, no FX-offline enqueue, no snapshot, and a
+  a saved project, a hand-edited chain and an undo/redo can each present one; the
+  Gotchas below enumerate the routes that still reach it. A park meeting that pair
+  REFUSES: no flag write, no FX write, no snapshot, and a
   console report NAMING the refused tracks (they are hidden from both the TCP and
   the mixer, so a count points at nothing the user can go and find) plus the hand
   recovery, printed once per changed refused set per project rather than on every
   reapply. Leaving the track as found is the only non-destructive answer once the
   pre-park truth is gone. **The refusal's cost is deliberate and it is not
-  symmetric with "hidden forever":** because it skips the FX enqueue too, a track
+  symmetric with "hidden forever":** because it skips the FX half too, a track
   whose flags read parked but whose FX are still online keeps them online for
   good. Refusing guarantees nothing false is written; it does not promise to
   finish the park. Decision at `view_fx_park`'s `decidePark`.
@@ -66,43 +66,51 @@ decide membership or mode rules.
   active mode — stateful plugins (convolution, loaded samplers, tail-holding
   effects) re-initialize on return (load hitch, un-persisted internal state lost).
   Accepted cost of the CPU reclaim; surfaced at the toggle affordance (tooltip).
-  **The hitch no longer sits on the switch's synchronous path:** per-FX
-  offline/online is enqueued and applied on a later idle tick (`view_fx_park`),
-  so the new mode paints first. The deferral changes only WHEN the plugins move —
-  they still unload and re-instantiate, and un-persisted internal state is still
-  lost. What it does change is the undo record. What was MEASURED, before the fix
-  below: a `TrackFX_SetOffline` made outside any undo block mints ONE implicit
+  **The hitch sits on the switch's synchronous path, deliberately.** Per-FX
+  offline/online runs inside `applyMode`'s own undo block, between the flag
+  writes and `Undo_EndBlock2`, so an action-driven switch does not return until
+  every plugin has moved. That cost is the price of the invariant below, and it
+  was accepted as such.
+- **ONE mode switch is ONE undo point.** Everything the switch wrote into the
+  project — the driven flags, every per-FX offline/online, the lane state —
+  rolls back in a single Ctrl-Z. (The `view_state` ext-state write is NOT among
+  them; it lands after the block, see Gotchas.) What was MEASURED (before any
+  fix): a `TrackFX_SetOffline` made outside any undo block mints ONE implicit
   undo point per call, and a small project showed 10 points for one switch — 1
-  for the switch itself and 9 for its FX — which Ctrl-Z then walked one FX at a
-  time. **The drain now opens its own undo block** (`fxParkUndoClose`,
-  `UNDO_STATE_FX` only, no other domain) on the PREMISE that an explicit block
-  suppresses those per-call implicit points. That premise, and everything below
-  that follows from it — one drain being one point, a switch costing two, a
-  no-write drain minting nothing (`fxParkUndoClose` owns why), and ONE Ctrl-Z
-  re-onlining every FX at once with a second rolling the flags and the ext state
-  back — is INFERENCE from the code's design, not observed: `docs/VERIFICATION.md`
-  §"Mode switching" (items 51–53) is where this gets checked against REAPER. The
-  ordering property the pre-fix measurement established is DELIBERATELY
-  PRESERVED BY DESIGN regardless: the drain's block is its own and never the
-  switch's, so the FX point is meant to sit ABOVE the switch point, with flags,
-  FX and snapshot moving together on undo. Folding the FX writes into the
-  switch's own block, or making them undo-silent, would break that and is not an
-  option here.
-  **The idle tick is not the only drain point.** Any path that
-  serializes the view model drains synchronously first (`persistViewState`,
-  `render_in_place`), because a save landing between a restore's synchronous flag
-  writes and its drain would record offline FX beside a model that no longer
-  carries the snapshot to replan them — unrecoverable on reopen. So an
-  action-driven switch does pay the FX hitch before it returns; the repaint and
-  the undo block have both closed by then, which is what the deferral was for.
-  Any new caller that reapplies a mode and then persists inherits this obligation.
+  for the switch itself and 9 for its FX. That measurement is the whole reason
+  the FX writes must sit inside a block. Which block is the design decision, and
+  it is settled: the SWITCH'S OWN. The undo surface
+  (`vendor/reaper-sdk/sdk/reaper_plugin_functions.h:7745-7822`) is
+  begin/end/state-change and nothing else — no append-to-point, no coalescing
+  flag, no undo-silent write — and a block cannot be held open across a return
+  to the message loop, so ANY deferral of the writes costs a second undo point.
+  Two points is a rejected design, not a lesser one: do not reintroduce a
+  deferred queue, an idle drain, or a second block. That an explicit block
+  suppresses the per-call implicit points is still the PREMISE the whole
+  arrangement rests on — INFERENCE from the measurement above, not observed;
+  `docs/VERIFICATION.md` §"Mode switching" is where it gets checked.
+- **A reapply that wrote nothing leaves no undo point.** The load-tick reapply
+  re-plans a park for every inactive leaf, and over a project saved fully parked
+  it finds every flag and every FX already where the plan wants them. The block
+  then closes on the `("", 0)` discard form (`view.h`'s `applyMintsUndoPoint`;
+  `mintManagedLanes`' no-op path uses the same idiom) — opening a project must
+  not cost the user a Ctrl-Z. A REAL switch always mints, written state or not:
+  it is an explicitly fired action and has to stay undoable even when the plan
+  found nothing to write.
 - **Stated DEVIATION — the undo mask does not keep FX out of a real switch.** The
-  apply mask (`kApplyUndoMask`) drops `UNDO_STATE_FX` and ORs it back in when a
-  driven flag in that domain moved; the only such flag is `I_FXEN`, which every
-  park writes. So any switch that parks at least one track still makes REAPER
-  marshal the project-wide FX chunk into its undo record. The saving is real only
-  on a reapply and on a no-op switch. Narrowing it further would mean not carrying
-  `I_FXEN` in the undo record at all, which would break "one switch is one Ctrl-Z."
+  apply mask (`kApplyUndoMask`) drops `UNDO_STATE_FX` and ORs it back in when
+  `I_FXEN` or a per-FX offline state actually moved — which every park does. So
+  any switch that parks at least one track still makes REAPER marshal the
+  project-wide FX chunk into its undo record. The saving is real only on a
+  reapply that wrote nothing and on a no-op switch. Narrowing it further would
+  mean not carrying the FX domain in the undo record at all, which would break
+  "one switch is one Ctrl-Z."
+- **A rapid A→B→A flip pays two full park/restore cycles.** The retired intent
+  queue let an intent annihilate its own pending inverse, so a fast double-toggle
+  moved no plugins at all. Synchronous writes have no such window: the first
+  switch offlines the chain, the second brings it back, and both hitches are
+  paid. Accepted — the flip was never free (the flag writes always ran twice),
+  and the alternative costs the one-point invariant above.
 - **Show-both semantics:** a per-track "pin visible across modes" flag re-enables
   processing whenever shown. A show-both leaf appears in every mode's visible set
   and is never parked — its driven flags stay at snapshot/restored values, FX
@@ -140,8 +148,8 @@ applies the resulting lane state to live tracks.
 
 ## Modules
 
-- `view` — Design View shell: snapshots flag values before parking, drives hide + CPU-park on inactive-mode leaves (`B_SHOWINTCP`/`B_SHOWINMIXER`/`B_MAINSEND`/`I_FXEN`, with per-FX offline deferred to `view_fx_park`), restores from snapshot. Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.**
-- `view_fx_park` — the park surface: the `TrackFX_GetFXGUID` identity read snapshot/park/restore share, the deferred intent queue that keeps `TrackFX_SetOffline` off the switch's synchronous path (at most one intent per track GUID, latest wins, an intent landing on its own pending inverse cancels it), the idle-tick drain `main.cpp`'s `OnTimer` calls and the ONE undo point it collapses its writes into (`fxParkUndoClose` — it opens that block itself, so no caller may drain while holding one), the per-track park decision (`decidePark` over "does the model hold a snapshot" × "does the chain already read parked", with `decideParkForTrack` owning the live reads BOTH halves of that second term needs, and `trackFlagParm` the ONE `Flag` → REAPER-parameter mapping snapshot's read, park's read and restore's write all address), and the refusal report (`reportRefusedParks`, which names the tracks and holds the last-reported set so a reapply does not reprint it). **The drain owns no model state.** A snapshot is dropped where the restore is PLANNED — the flags are back at their captured values from that moment, and a model that still described the track as parked would let a persist or a reapply inside the drain window replan a restore over whatever the user changed since. What the deferral costs instead is that the live FX chain stops being a trustworthy snapshot source while an intent is pending: a park that CANCELS a pending restore takes the pre-park FX states from that restore's ops (`preParkFxFromCancelledRestore`), because the chain still reads the parked values and the cancel means no drain will ever fix them.
+- `view` — Design View shell: runs the pure planner over the live folder tree and iterates its park/restore plans, applies managed-lane and parent-visibility writes, and owns the ONE undo block all of that rides (`applyMintsUndoPoint` decides whether it leaves a point). Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.**
+- `view_fx_park` — the park surface: ONE track's whole move, both halves together. `parkTrack` is the trust decision (`decidePark` over "does the model hold a snapshot" × "does the chain already read parked", with `decideParkForTrack` owning the live reads BOTH halves of that second term needs), the pre-park snapshot when it is owed, then flags before FX; `restoreTrack` mirrors it, FX before flags, and consumes the snapshot. Each reports back what the caller's block needs (`TrackApplyResult`: did anything move, does the mask owe `UNDO_STATE_FX`, was the park refused, what could a restore not place). Also the `TrackFX_GetFXGUID` identity read all three of snapshot/park/restore address FX through, `trackFlagParm` — the ONE `Flag` → REAPER-parameter mapping they likewise share — the `writeIfChanged` read-before-write every driven parameter goes through, `trackStillLive` (a per-track `ValidatePtr2`, because a plugin unloading is assumed to pump the message loop), and the two console reports (`reportRefusedParks`, which names the tracks and holds the last-reported set so a reapply does not reprint it; `reportFxRestoreDrops`). **The snapshot is consumed where the restore RUNS** — from that line the flags are back at their captured values, and a model still describing the track as parked would let a later persist or reapply replan a restore over whatever the user changed since.
 - `view_solo` — the `I_SOLO` read/write pair behind the per-mode solo surface, plus `clearTrackSolos`/`restoreTrackSolos`, the outgoing-clear and incoming-replay entry points `view` drives them through. Holds no policy: what to cache, clear, or replay is `core/view/solo_cache`.
 
 ## Gotchas
@@ -159,42 +167,60 @@ applies the resulting lane state to live tracks.
   under whatever mode id is active at that point, not the one the user undid back
   to. Pre-existing: `snapshots_` already carries this same model-vs-undo split;
   the solo cache inherits it rather than introducing it. Not fixed here.
-- An undo/redo also DISCARDS every pending FX intent (`discardDeferredFxParks`),
-  which is not the pure loss it reads as: the same tick reapplies the active mode
-  over the reloaded model, re-planning a park for every inactive leaf, so parked
-  FX converge on the following drain. The one case that does not self-heal is a
-  track whose reloaded model carries no snapshot — nothing plans its restore, so
-  FX left offline stay offline, and its next park REFUSES rather than
-  re-snapshotting the parked chain (snapshot-source invariant above). Stuck, but
-  never falsely committed — and never finished either, since the refusal skips the
-  FX enqueue too. Full contract at `discardDeferredFxParks`.
-- **Which routes actually reach that no-snapshot-plus-parked-chain pair.** An undo
-  of a mode switch (Ctrl-Z) is NOT one of them, ON THE UNDO-ORDERING PREMISE
-  above (`[verify — DAW]`, not yet observed — see Invariants): if the drain's
-  point does sit above the switch's, every FX is back online by the step that
-  rolls the flags and the ext state back — and the step in between (FX online,
-  flags still parked) still holds the model's snapshot, which decides
-  `ParkOnly`, not a refusal. Whether the ext-state
-  snapshot comes back with them is INFERENCE, not observed, and untested in the
-  REDO direction: `[verify — DAW]` does a redo that re-parks a track's flags/FX
-  also restore a `view_state` that is snapshot-free for it (matching a fresh
-  park), or can it re-park the live track while the model still holds a stale
-  snapshot? What is confirmed to reach the pair: a `view_state` that PARSED but
-  carries no snapshot for the track (a snapshot `reconcile` pruned while its
-  track was out of the live enumeration, then undo/redo-restored with the track
-  but not the snapshot), and a hand or script edit that leaves a chain
-  park-shaped — including the single likeliest real case, a track with untouched
-  flags and just ONE hand-offlined plugin (the FX half of the disjunction
-  refuses alone; the flags never have to read parked too). A track the USER
-  keeps hidden from both panels, out of the mix and FX-bypassed reads
-  identically to all of them and is refused too — no lost state there at all,
-  which is why the report asserts no cause.
-  A DIFFERENT strand entirely, which the refusal report does NOT cover: an
+- **Which routes reach the no-snapshot-plus-parked-chain pair, re-derived for the
+  one-point design.** Three of the routes that used to reach it were artifacts of
+  the retired deferral window and are GONE BY CONSTRUCTION: there is no longer a
+  moment in which a track's flags have been written but its FX have not, so no
+  save, no reload and no discard can land between the two halves, and there is no
+  queued intent for an undo to drop. What survives:
+  - A `view_state` that PARSED but carries no snapshot for the track — a snapshot
+    `reconcile` pruned while its track was out of the live enumeration, then
+    undo/redo-restored with the track but not the snapshot. (Asserted by the
+    branch that introduced the refusal; not re-observed since.)
+  - A hand or script edit that leaves a chain park-shaped — including the single
+    likeliest real case, a track with untouched flags and just ONE hand-offlined
+    plugin (the FX half of the disjunction refuses alone; the flags never have to
+    read parked too).
+  - A track the USER keeps hidden from both panels, out of the mix and
+    FX-bypassed. It reads identically to both of the above and is refused too —
+    no lost state there at all, which is why the report asserts no cause.
+
+  An undo of a mode switch (Ctrl-Z) is NOT a route, and no longer needs an
+  ordering premise to say so: flags and FX moved inside ONE block, so they roll
+  back together and no intermediate state exists for a park to misread.
+- **The `view_state` write is NOT inside `applyMode`'s block.** `persistViewState`
+  (`design_view_actions.cpp`) runs after `applyMode` has returned, so
+  `SetProjExtState` lands past `Undo_EndBlock2`. The UNDO direction still comes
+  out right — the point closed over the ext state as it stood BEFORE the switch,
+  which is what a Ctrl-Z should restore — but by position, not by design. The
+  REDO direction is what this leaves open: the point never saw the new
+  `view_state`, so a redo may re-park the tracks while restoring the model that
+  describes the mode the user undid back to. `[verify — DAW]`; if it does bite,
+  the result is the pre-existing model-vs-undo split the solo-cache gotcha above
+  already describes, not a new refusal route. Do not "fix" it by moving
+  `persistViewState` inside the block — it can open a modal Save-As.
+- A DIFFERENT strand entirely, which the refusal report does NOT cover: an
   absent or malformed `view_state` (`loadViewModel`, `ext_state_io.cpp`) is not
   a parked-chain reading at all — it falls back to a DEFAULT model (active mode
   Arrange, empty membership), so nothing is planned to park the track,
   `decidePark` never runs, and a track saved parked stays stuck with no console
   line whatsoever.
+- **`applyMode`'s block can be nested, and now carries FX writes while it is.**
+  `render_in_place` calls `applyMode` inside its own `Undo_BeginBlock2` pair, so
+  that verb's single `UNDO_STATE_ALL` point now absorbs the reapply's FX moves
+  instead of a separate point landing after it — the better outcome, but it rests
+  on REAPER coalescing nested blocks, which the SDK header does not document.
+  The inverse is also open: a plugin unloading is ASSUMED to pump the message
+  loop, so an action fired mid-apply could open its own block inside this one.
+  Nothing guards against that (the retired queue's `g_draining` never did either
+  — it only stopped a second drain). Both are `[verify — DAW]`.
+  That same premise leaves a STATED RESIDUAL on track handles. The park and
+  restore loops re-`ValidatePtr2` per track (`trackStillLive`), because they are
+  the ones interleaved with the FX writes. The passes that run AFTER them — lane
+  ops, parent visibility, the solo replay — still use handles resolved before the
+  first FX write, so a track deleted mid-apply would reach them stale. Not closed:
+  the premise it depends on is itself unobserved, and a partial guard covering
+  three of those four passes would read as complete.
 - `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
   attached to its plugin across a chain mutation while parked. That is
   `[verify — DAW]` (see `fxGuidString` in `view_fx_park.cpp`) and SWS issue #802 is a