Close pass four: undetented host curve read, LiveValues guard, ordering assert, static-lane fix, docs

Points toPlain's exponent arm at the undetented curve map so host reads
match the editor; adds a sizeof guard plus field-poison test for
LiveValues::operator==; skips the model write when an automation value
hasn't moved; corrects five stale doc citations.

src/shell/instrument/CLAUDE.md

@@ -126,14 +126,18 @@ the single authority.** Everything else that holds these values is a cache or a
 | Host controller write (`setParamNormalized`) | the call | the call returns (it writes the model) |
 | State restore (`setState`) | the call | the call returns |
 | Bake reset (`adoptBakedCapture`) | the call | the call returns |
-| Reload seed (`reloadInstrument`) | under `reloadMutex_` | the publish (it re-folds the model) |
+| Limiter toggle (`setLimiterEnabled`) | the call | the call returns (it writes the model too, but through neither `commitLive` nor `commitAndReload`) |
+| Reload seed (`reloadInstrument`) | never — it does not write `params_` | it only republishes a live block folded from whatever the model already holds |
 | **Host automation point** (`IParameterChanges`) | the block it lands in | **the UI thread has folded it into the model and republished** |
 
-Every writer except the last writes the model directly, so for those "authority ends" is just
-"the write happened". The automation lane is the only one that cannot: the SDK delivers it on the
-audio thread, where the model path allocates (`resolvePlay` copies velocity curves and spline
-contours). So it patches the engine-facing block in place and is couriered to the UI thread,
-which folds it into the model on the next tick.
+Every writer above except the last two writes the model directly, so for those "authority ends"
+is just "the write happened". Reload seed is not itself a model write — `reloadInstrument` never
+touches `params_`; the only write in the tree is `setInstrumentParams`'s, `processor_state.cpp:192`
+— which is why its row states no authority window of its own. The automation lane is the only one
+that cannot write directly: the SDK delivers it on the audio thread, where the model path
+allocates (`resolvePlay` copies velocity curves and spline contours). So it patches the
+engine-facing block in place and is couriered to the UI thread, which folds it into the model on
+the next tick.
 
 **The hold is the bridge across that gap, and nothing more.** Between the point landing and the
 fold — at most one UI tick — the model does not yet carry the value, so a model republish in that
@@ -171,14 +175,15 @@ under it: the automation fold, the host's generic panel, a state restore.
 different windows: `AutomationChannel::land` drops a repeat of a standing hold whole (the flat
 read-mode segment, where a host sends one point per block), and the merge publishes only when the
 merged block differs from the last (a model republish that changed nothing). Neither is measured
-against a performance budget — they are there because `VoiceEngine::refreshLive` runs
+against a performance budget — they are there because `VoiceEngine::applyLiveToActive` runs
 `voice.applyLive` over every active voice, and neither case needs it.
 
 - **The automation values fold back into the model on the UI thread** (`drainAutomationToModel`,
-  called from `getState`, the editor's sync tick, and the bake's reload tail). The blob is
-  authoritative, so a value that never came back would be lost on save. The fold is suppressed
-  from notifying the host — the values came FROM it, and echoing them would let a lane in write
-  mode re-record its own playback.
+  called from `getState`, the editor's sync tick, and `instrument_bake.cpp:125` — at the HEAD of
+  the bake chain, before the render, not its reload tail). The blob is authoritative, so a value
+  that never came back would be lost on save. The fold is suppressed from notifying the host —
+  the values came FROM it, and echoing them would let a lane in write mode re-record its own
+  playback.
 - **`IMidiMapping` is deliberately NOT implemented** — no conventional CC names most of what
   is exposed, an invented map would hijack CCs the user's controller already sends, and
   `[verify — DAW]` REAPER's own per-parameter MIDI learn is expected to cover the case without

src/shell/instrument/automation_channel.h

@@ -2,6 +2,9 @@
 // AUTHORITY LIFETIME is mechanised: a point outranks the model from the block it lands in until
 // the UI thread has folded it back into the model AND republished. This directory's CLAUDE.md
 // states the model; `core/instrument/param/param_merge` is the pure decision this feeds.
+// The release itself is observed the NEXT BLOCK, not the instant it happens — refreshReleases()
+// only runs inside process()'s merge branch (reasampler_processor.cpp), and publishLiveParams
+// always bumps the generation that branch checks, so the next block is guaranteed to take it.
 
 #pragma once
 
@@ -44,10 +47,13 @@ public:
         return ridesTheBlock;
     }
 
-    // Refreshes each held slot's release answer. Must run BEFORE the model block is read: the
-    // acquire here synchronizes with the UI thread's release store, which it makes only AFTER
-    // republishing the model — so a slot seen released is one whose value any block read after
-    // this point is guaranteed to already carry.
+    // Refreshes each held slot's release answer. Runs only inside process()'s merge branch, so a
+    // release lands the NEXT BLOCK after the UI thread makes it, never the same instant — benign,
+    // because publishLiveParams always bumps the generation that branch checks, so the next block
+    // is guaranteed to run this. Must run BEFORE the model block is read: the acquire here
+    // synchronizes with the UI thread's release store, which it makes only AFTER republishing the
+    // model — so a slot seen released is one whose value any block read after this point is
+    // guaranteed to already carry.
     void refreshReleases() {
         for (std::size_t i = 0; i < kDeckParamSlots; ++i) {
             if (!slots_[i].held) continue;

src/shell/instrument/editor_session.cpp

@@ -211,10 +211,11 @@ void ReaSamplerEditor::commitAndReload() {
     // self-contained for that sample.
     if (!processor_) return;
     processor_->setSelectedSampleId(selectedId_);
-    processor_->setInstrumentParams(params_);
-    // This copy IS the model now, so adopt the generation it produced rather than re-seeding off
-    // it on the next tick. Same reason at commitLive.
-    seenParamsGeneration_ = processor_->instrumentParamsGeneration();
+    // This copy IS the model now, so adopt the generation IT produced (the return, not a second
+    // separate query — a write landing between the two would make this adopt a generation newer
+    // than the copy just sent) rather than re-seeding off it on the next tick. Same reason at
+    // commitLive.
+    seenParamsGeneration_ = processor_->setInstrumentParams(params_);
     processor_->reloadInstrument();
     // The reload may have auto-defaulted the channel mode (implicit only) — re-read so the
     // toggle draws what the engine actually decoded with.
@@ -227,8 +228,7 @@ void ReaSamplerEditor::commitAndReload() {
 void ReaSamplerEditor::commitLive() {
     // UI thread only. See the declaration for why this still writes the parameter set.
     if (!processor_) return;
-    processor_->setInstrumentParams(params_);
-    seenParamsGeneration_ = processor_->instrumentParamsGeneration();
+    seenParamsGeneration_ = processor_->setInstrumentParams(params_);
     processor_->publishLiveParams();
 }
 

src/shell/instrument/instrument_params.cpp

@@ -6,6 +6,8 @@
 
 #include "shell/instrument/reasampler_processor.h"
 
+#include <cassert>
+
 #include "base/source/fstring.h"
 #include "pluginterfaces/base/ustring.h"
 #include "pluginterfaces/vst/ivstparameterchanges.h"  // IParameterChanges / IParamValueQueue
@@ -291,16 +293,23 @@ void ReaSamplerProcessor::drainAutomationToModel() {
         ++foldedCount;
         // Master gain's model IS the atomic the audio thread already wrote; there is nothing to
         // fold, only the controller cache to refresh below.
-        if (row.deck != DeckParam::kMasterGain) {
-            writeDeckParamToModel(params, row.deck, value);
-            moved = true;
-        }
+        if (row.deck == DeckParam::kMasterGain) continue;
+        // A present-but-static lane resends the SAME point every tick; writing it back would
+        // republish liveParams_ and move paramsGeneration_ — a full model copy, a controller
+        // cache write and an editor repaint, every tick, forever, for a value that never moved.
+        // The release below still fires: the model already carries the point either way.
+        if (value == modelParamNormalized(params, row.deck)) continue;
+        writeDeckParamToModel(params, row.deck, value);
+        moved = true;
     }
     // Suppressed for the whole fold: these values CAME from the host, and echoing them back
     // through performEdit would let a lane in write mode re-record its own playback. The
     // controller cache is still refreshed, so the host's display and the editor follow.
     const bool wasSuppressed = paramNotifySuppressed_;
     paramNotifySuppressed_ = true;
+    // Captured before the publish below, so the assert at the release loop can tell "the model's
+    // publish already landed" from "it merely happened to be in flight for some other reason".
+    const std::uint32_t generationBeforeFold = liveParams_.generation();
     if (moved) {
         setInstrumentParams(params);
         publishLiveParams();
@@ -311,6 +320,13 @@ void ReaSamplerProcessor::drainAutomationToModel() {
     // model carries the point. Released any earlier and the audio thread could drop the hold
     // ahead of the block that carries its value; never released at all — the defect this
     // replaces — and one point would defeat every later restore, reset and knob move.
+    // Enforced, not just commented: two prior passes inverted this order and every test in the
+    // tree still passed, because nothing exercises `process()`. `publishLiveParams` is a no-op
+    // before the engine has a sample rate (`builtSampleRate_`), which is the one case this assert
+    // must not fire for.
+    assert((!moved || builtSampleRate_.load(std::memory_order_relaxed) <= 0 ||
+            liveParams_.generation() != generationBeforeFold) &&
+           "release ran ahead of the publish that gives it authority");
     for (std::size_t i = 0; i < foldedCount; ++i) {
         automation_.release(foldedSlots[i], foldedSeqs[i]);
     }

src/shell/instrument/processor_state.cpp

@@ -179,11 +179,12 @@ InstrumentParams ReaSamplerProcessor::instrumentParams(std::uint32_t& generation
     return params_;
 }
 
-void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) {
+std::uint32_t ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) {
     // The exposed values alone, not the whole set: this funnel fires per mouse move on every live
     // knob and node drag, and InstrumentParams owns seven vectors — copying all of them to diff
     // 44 doubles is the cost, and the diff is what the notification actually needs.
     double before[kDeckParamSlots];
+    std::uint32_t generation;
     {
         std::lock_guard<std::mutex> lock(paramsMutex_);
         for (const instrument::param::ParamRow& row : instrument::param::exposedParams()) {
@@ -191,8 +192,10 @@ void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) {
         }
         params_ = params;
         // Bumped inside the lock with the write it names, so a reader taking the pair together
-        // can never see a generation that does not describe the set beside it.
-        ++paramsGeneration_;
+        // can never see a generation that does not describe the set beside it. Read back before
+        // the unlock for the same reason — a caller wanting ITS OWN write's generation must not
+        // race a second writer's bump between this function's unlock and its return.
+        generation = ++paramsGeneration_;
     }
     // Every writer of the parameter set — setState, the editor's commits, the bake's adopt —
     // funnels through here, so mirroring the limiter flag at this one point is what keeps the
@@ -214,6 +217,7 @@ void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) {
     // limiter mirror sits here: an internal write that skipped it would leave the host
     // displaying — and, on the next touch, re-imposing — the superseded value.
     notifyParamsFromModel(before, params);
+    return generation;
 }
 
 void ReaSamplerProcessor::flushLatencyRestart() {

src/shell/instrument/reasampler_processor.h

@@ -190,7 +190,7 @@ public:
     // never read on the audio thread — reloadInstrument bakes it into the SampleData
     // off-thread.
     InstrumentParams instrumentParams();
-    void setInstrumentParams(const InstrumentParams& params);
+    std::uint32_t setInstrumentParams(const InstrumentParams& params);  // adopt THIS generation, never a second instrumentParamsGeneration() call
 
     // The model's edit counter, bumped by every setInstrumentParams. A holder of a COPY (the
     // editor's snapshot) re-seeds when this moves under it; the overload answers both under one
@@ -392,9 +392,9 @@ private:
     // and republishes ONLY when the RESULT moved — so neither an unchanged model nor a lane
     // resending the value it already sent reaches the per-voice fan-out. A block carrying no
     // automation and no model change costs one relaxed load plus, when the host passed a non-null
-    // IParameterChanges (REAPER's normal case), one cross-module getParameterCount(). Two blocks
-    // because the seqlock's single-writer contract is load-bearing and the two writers differ in
-    // thread; this directory's CLAUDE.md owns the argument.
+    // IParameterChanges (`[verify — DAW]` REAPER's normal case), one cross-module
+    // getParameterCount(). Two blocks because the seqlock's single-writer contract is load-bearing
+    // and the two writers differ in thread; this directory's CLAUDE.md owns the argument.
     instrument::engine::LiveParams automationLive_;
     // Audio thread only. The last liveParams_ generation merged, the last block published (the
     // republish gate compares against it), and the automation slots themselves.