Remediate Phase P render-in-place review findings

Fix the ProjectMedia refusal path's false bank claims and file relocation, an unreachable-undo idiom, and eight comment/doc accuracy issues.

src/core/capture/capture_name.cpp

@@ -90,8 +90,10 @@ std::string captureTrackName(const std::string& sourceName) {
     const std::string prefix(kCaptureTrackPrefix);
     // A source with no readable name yields the bare word rather than a trailing
     // space; both spellings are fixed points, which is what makes the whole function
-    // one (a track named exactly "Capture" must not become "Capture Capture").
-    const std::string bare = prefix.substr(0, prefix.size() - 1);
+    // one (a track named exactly "Capture" must not become "Capture Capture"). Read
+    // from kCaptureTrackPrefixBare rather than chopped off prefix, so the two names
+    // can't drift out of sync with each other (both expand from the same header token).
+    const std::string bare = kCaptureTrackPrefixBare;
 
     if (sourceName.empty()) return bare;
     if (sourceName == bare) return sourceName;

src/core/capture/capture_name.h

@@ -59,10 +59,20 @@ std::string formatCaptureStamp(const CaptureStamp& stamp);
 
 CaptureName composeCaptureName(const CaptureNameInputs& in);
 
+// The single source of truth for the word itself — kCaptureTrackPrefixBare and
+// kCaptureTrackPrefix below both expand from this one token, so editing it can never
+// desync captureTrackName's "no readable source name" bare-word fallback from the
+// separator-terminated prefix it is derived from.
+#define REASAMPLER_CAPTURE_TRACK_WORD "Capture"
+
+// The bare word behind kCaptureTrackPrefix, needed by captureTrackName's
+// no-readable-source-name fallback.
+inline constexpr const char* kCaptureTrackPrefixBare = REASAMPLER_CAPTURE_TRACK_WORD;
+
 // Prefixed onto a source track's name to name the track a render-in-place created.
 // A display convention, not a persisted key — unlike a lane prefix or an action-id
 // suffix, changing it later strands nothing.
-inline constexpr const char* kCaptureTrackPrefix = "Capture ";
+inline constexpr const char* kCaptureTrackPrefix = REASAMPLER_CAPTURE_TRACK_WORD " ";
 
 // The new track's name for a render of `sourceName`. IDEMPOTENT — a fixed point on
 // its own output, so a second render over a result track yields "Capture MONEY"

src/core/capture/track_topology.h

@@ -43,8 +43,10 @@ struct SiblingPlacement {
 // Levels are absolute nesting depths recovered from the deltas (level[0] = 0,
 // level[i+1] = level[i] + depth[i]). A folder parent's insert point is the first
 // following track back at the source's own level — i.e. after the whole folder;
-// everything else inserts directly below the source. The two writes preserve the
-// total delta sum, so no track after the insertion changes level.
+// everything else inserts directly below the source. On a well-formed delta list
+// (one whose deltas sum to zero) the two writes preserve the total delta sum, so no
+// track after the insertion changes level — the malformed case below does not carry
+// that guarantee; the clamp keeps the result legal, not level-preserving.
 //
 // A malformed list (deltas not summing to zero, an out-of-range srcIndex) CLAMPS to
 // the nearest legal placement rather than asserting: the failure mode of a corrupt

src/core/view/CLAUDE.md

@@ -64,8 +64,8 @@ settled 2026-07-23):
 - **Mechanism: fixed item lanes.** Map mode → lane; toggle drives per-lane
   play/show so only the active mode's lane is present. Items keep their real
   position and real track — nothing is moved in time or deleted.
-- **Membership: adoption rule for new items; active mode for new tracks.** New
-  tracks are tagged to the active mode at creation **only when the GUID carries no
+- **Membership: adoption rule for new items; active mode for new tracks absent an
+  explicit tag.** New tracks are tagged to the active mode at creation **only when the GUID carries no
   membership record** — an explicit tag wins over the detector, because the detector
   classifies content the *user* made, not content the tool made and already
   classified. New items follow an

src/shell/capture/CLAUDE.md

@@ -55,12 +55,12 @@ detail not covered there:
 
 ## Modules
 
-- `capture` — two CONCRETE backends with deliberately different lifecycles (no shared interface — the former `ICaptureBackend` was deleted in Q-W3, T4-26: one deriver, zero polymorphic call sites): `OfflineRenderBackend` (deterministic default, synchronous) and `RealtimeRecordBackend` (async begin/tick/abort). Input: `CaptureRequest`. Output: finished file + populated `Sample` handed to `bank_model`. It also owns the two file-side steps both backends share, in this order: `collapseCapturedFileToMono` (the lossless mono collapse, applied to the landed file) and `stampCaptureSample`, which measures the channel count off that same file so the entry and the audio cannot disagree. And `captureNameFor` — the impure local-clock read the entry points call to build a request's label + stem, kept out of the pure `core/capture/capture_name` composition it feeds.
+- `capture` — two CONCRETE backends with deliberately different lifecycles (no shared interface — the former `ICaptureBackend` was deleted in Q-W3, T4-26: one deriver, zero polymorphic call sites): `OfflineRenderBackend` (deterministic default, synchronous) and `RealtimeRecordBackend` (async begin/tick/abort). Input: `CaptureRequest`. Output: finished file + populated `Sample` — destination-dependent: on `CaptureDestination::Bank` (the default) the `Sample` is handed to `bank_model`; on `ProjectMedia` the file lands outside the bank and the caller (`render_in_place`) discards the returned `Sample`. It also owns the two file-side steps both backends share, in this order: `collapseCapturedFileToMono` (the lossless mono collapse, applied to the landed file) and `stampCaptureSample`, which measures the channel count off that same file so the entry and the audio cannot disagree. And `captureNameFor` — the impure local-clock read the entry points call to build a request's label + stem, kept out of the pure `core/capture/capture_name` composition it feeds.
 - `render_bounds_gate` (`shell/capture`) — the exact-bounds verdict on a landed offline render and the refusal's file handling, split off `capture.cpp` on the render-vs-judge seam. Refuses a frame count that is not the window's AND a file whose frames cannot be measured at all (an invalid layout used to skip the gate and land with an unknown channel count). Judges `TailMode::None` only — Auto/Manual add frames by design, and an unmeasurable render still lands under those two (`docs/TODO.md`). A refused render is MOVED to `<projectDir>/reasampler_refused/` rather than deleted, so the frames it did print survive for diagnosis while the short-render root cause is open; the bank never INDEXES it either way — but a failed move leaves the file sitting unindexed in the bank folder itself, not `reasampler_refused/` (the console message says which happened).
 - `scope_resolve` (`shell/capture`) — scope/source resolution shared by every capture entry point (Q-W3 hoist out of `main.cpp`): razor-else-time range inference, selected-track/selected-item-owning-track collection with canonical GUIDs, and the M10 provenance-assembly inputs (read BEFORE the FX-bypass guard neutralizes the in-scope chain). Also the one place a source track's NAME is read (`trackName`, via `GetTrackName` — chosen over `P_NAME` because it already answers REAPER's `"Track N"` convention for an unnamed track), landed on `ResolvedSource::trackNames` parallel to `sourceTracks` and composed into the capture's label + stem by the pure `core/capture/capture_name`.
 - `render_selection` (`shell/capture`) — the transient track selection a selected-tracks render (`&128`) requires, as a stack RAII guard: REAPER prints whatever tracks are selected, so `renderOffline` makes the request's own tracks BE the selection for the render's duration and restores the user's set on every exit path. Engaged ONLY for that source mode, which leaves a stated residual: a `&32` selected-items render still prints whatever ITEMS the user has selected. Live captures are unaffected (that selection is the source), but a recipe replay of a `SelectedItems` capture renders against whatever happens to be selected then — the recipe stores tracks and a range, never item GUIDs, so this guard cannot close it. Filed in `docs/TODO.md`.
 - `render_isolation` (`shell/capture`) — the transient upstream silencing a ranged ITEM render needs, as a stack RAII guard alongside the two above: the selected-tracks source prints everything flowing INTO the track, so each direct folder child's `B_MAINSEND` and each of the track's receives' `B_MUTE` are cut for the render and restored on every exit path. Direct children only — a grandchild reaches the track through the child that owns it. The child-set walk is pure (`core/capture/track_topology`).
-- `capture_orchestrator` (`shell/capture`) — single-capture orchestration + the realtime/insert action bodies (Q-W3 hoist, T4-02): `renderOffline` (one offline render under the scope's FX-bypass guard), `captureAndIndexOne` (render + provenance stamp + bank add + tracking-ledger record, unpersisted), `RunCapture`/`RunCaptureItemAssign`, `RunCaptureRealtimeTrack`/`RunCancelRealtime` (the realtime action bodies — the in-flight state lives in `realtime_lifecycle`), and `RunInsertSelected` (the ONE deliberate exception to capture-never-places).
+- `capture_orchestrator` (`shell/capture`) — single-capture orchestration + the realtime/insert action bodies (Q-W3 hoist, T4-02): `renderOffline` (one offline render under the scope's FX-bypass guard), `captureAndIndexOne` (render + provenance stamp + bank add + tracking-ledger record, unpersisted), `RunCapture`/`RunCaptureItemAssign`, `RunCaptureRealtimeTrack`/`RunCancelRealtime` (the realtime action bodies — the in-flight state lives in `realtime_lifecycle`), and `RunInsertSelected` (the capture family's deliberate exception to capture-never-places — see the Invariants section above for `render_in_place`, the directory's other placing path, which sits outside the capture family entirely).
 - `bake_land` (`shell/capture`) — the EXTENSION's half of the resample chain, the SCAN PASS: scans every open project tab for pending `rsbake_*` requests, lands the ones belonging to the project this session has loaded (via `bake_landing`, below), and refuses the rest with `WrongProject` — one undo point for the batch, each answered over its own key inside the invoking instance's synchronous action call. It owns every ext-state read and write in the chain. The per-key verdict itself is NOT this TU's: it is `core/wire`'s pure `classifyBakeScan`, so this shell only enumerates, reads, and applies — counting every verdict into a `wire::BakeScanTally` as it goes, printing `wire::describeBakeKey` for EVERY enumerated key (the only thing that names which key is whose) plus `wire::describeBakeScan` whenever any key went unanswered or any answer's write was not confirmed, in one `ShowConsoleMsg`. It PROVES every write — answer or stale-clear — by reading the key back (`wire::extStateWriteLanded`, whose home is `core/wire/ext_state_read.h`); an answer that did not land is the one no-answer the tally alone cannot show. That proof is three-valued (`wire::BakeWriteProof`): a read-back that overflowed, or a throw AFTER the `SetProjExtState` call, reports Unknown; a throw BEFORE it reports Rejected, because the write is then known not to have been made. Each key is materialized before any answer is written, so no `SetProjExtState` in this action mutates a set the enumerator is still walking. Answers are held UNENCODED until after the pass's single persist, so a landing whose pass never got its persist through is answered as a failure rather than as an `Ok` no reload would honour — `wire::bakeLandingAfterPersist` is the ONE route to a `Banked` landing, and no path here (dedup included) may assign that word itself. The undo block is stack RAII (`UndoBlock`). Both loops are guarded: a throw in the scan still writes the answers already prepared, and a throw in the write-back loop still prints the lines already accumulated — no path through this action can end in a silent console. It RENDERS NOTHING — the instrument already did, through its own engine in its own process, which is what makes the baked audio the sound the user approved and what keeps the voice engine out of the extension's link graph.
 - `bake_landing` (`shell/capture`) — landing ONE bake request, split off `bake_land` on the one-request / whole-pass seam; touches no REAPER API at all. Non-mutating `prepareLanding` and mutating `commitLanding` sit under separate catches in `attemptLanding` — a throw before anything was written is a clean refusal, a throw after it is reported as possibly partial. Replace-vs-add comes from `tracking::resampleLanding`; a replace keeps the entry's id and slot and never deletes the superseded file. Hash-dedup applies on the add path only, before the disk write, matching `updateSampleInPlace`'s "an in-place refresh is not an insert" — and a dedup hit still rides the pass's persist, because the entry it points at may be one the same pass just added. A refused index withdraws the bytes this call had just written — the self-cleanup carve-out from prune's deletion authority, stated in `prune_fs.cpp`'s header. It never persists: the pass does that once for its whole batch, which is why no landing may report itself as banked.
 - `capture_batch` (`shell/capture`) — the batch-capture family + re-capture-from-source (Q-W3 hoist, T4-02): `RunBatchCaptureItems` (one sample per selected item), `RunBatchCaptureRazor` (one sample per razor area), `RunRecaptureFromSource` (regenerate a provenanced sample from its recorded source's current state, bank-only). Every unit routes through `capture_orchestrator` so every precision invariant holds; persist is batched to one ext-state write per action.

src/shell/capture/capture.cpp

@@ -561,7 +561,7 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) {
     // legitimately produce zero (docs/TODO.md "0-byte render" entry: before this check,
     // Auto/Manual landed an empty file as CaptureStatus::Ok with channelCount == 0).
     const BoundsVerdict emptyVerdict =
-        checkRenderedFileNotEmpty(expectedPath, projectDir);
+        checkRenderedFileNotEmpty(expectedPath, projectDir, request.destination);
     if (emptyVerdict.refused) {
         result.status  = CaptureStatus::BoundsMismatch;
         result.message = emptyVerdict.message;

src/shell/capture/render_bounds_gate.cpp

@@ -45,6 +45,25 @@ std::string retainRefusedRender(const std::string& renderedPath,
            renderedPath + ", indexed by nothing. Delete it when done.";
 }
 
+// ProjectMedia is the project's own media, never the bank's (docs/product/render-in-place.md
+// "Where the file goes") -- a refusal takes no custody of it. No move, no bank folder, no
+// mention of a bank the render was never headed for.
+std::string leaveRefusedRenderInPlace(const std::string& renderedPath) {
+    return " The render was left where it was written, at " + renderedPath +
+           " -- delete it when done.";
+}
+
+// Dispatches the refusal's file-handling sentence by destination, so both verdict
+// functions below state one true thing about the file rather than the bank sentence
+// on every destination.
+std::string refusalOutcome(const std::string& renderedPath,
+                           const std::string& projectDir,
+                           CaptureDestination destination) {
+    if (destination == CaptureDestination::ProjectMedia)
+        return leaveRefusedRenderInPlace(renderedPath);
+    return retainRefusedRender(renderedPath, projectDir);
+}
+
 }  // namespace
 
 std::string refusedRenderFolder(const std::string& projectDir) {
@@ -52,7 +71,8 @@ std::string refusedRenderFolder(const std::string& projectDir) {
 }
 
 BoundsVerdict checkRenderedFileNotEmpty(const std::string& renderedPath,
-                                        const std::string& projectDir) {
+                                        const std::string& projectDir,
+                                        CaptureDestination destination) {
     BoundsVerdict v;
     std::error_code ec;
     const std::uintmax_t size = std::filesystem::file_size(renderedPath, ec);
@@ -61,7 +81,7 @@ BoundsVerdict checkRenderedFileNotEmpty(const std::string& renderedPath,
     v.refused = true;
     v.message = "Render at " + renderedPath + " is 0 bytes -- REAPER produced an empty "
                 "file, so there is nothing to check the requested range against." +
-                retainRefusedRender(renderedPath, projectDir);
+                refusalOutcome(renderedPath, projectDir, destination);
     return v;
 }
 
@@ -89,7 +109,7 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath,
                     "not be read (locked, missing, or a permissions error), its WAV "
                     "header did not parse, or it declared no sample rate -- so the "
                     "frames it holds were never checked against the requested range." +
-                    source + retainRefusedRender(renderedPath, projectDir);
+                    source + refusalOutcome(renderedPath, projectDir, request.destination);
         return v;
     }
 
@@ -124,7 +144,7 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath,
                 std::to_string(request.endSeconds) + "s) -> frame indices [" +
                 std::to_string(std::llround(request.startSeconds * rate)) + ", " +
                 std::to_string(std::llround(request.endSeconds * rate)) + ")." +
-                msNote + retainRefusedRender(renderedPath, projectDir);
+                msNote + refusalOutcome(renderedPath, projectDir, request.destination);
     return v;
 }
 

src/shell/capture/render_bounds_gate.h

@@ -10,9 +10,11 @@
 
 namespace reasampler::capture {
 
-// A refused render is MOVED out of the bank, not deleted: while the root cause of a
-// short render is open (docs/TODO.md), the frames it did print are the evidence — and
-// nothing may index a file the bank never accepted.
+// On the Bank destination, a refused render is MOVED out of the bank, not deleted: while
+// the root cause of a short render is open (docs/TODO.md), the frames it did print are
+// the evidence — and nothing may index a file the bank never accepted. On ProjectMedia,
+// the render is the project's own media (docs/product/render-in-place.md "Where the file
+// goes"), so a refusal leaves it exactly where it was written — no move, no bank folder.
 struct BoundsVerdict {
     bool        refused = false;
     std::string message;   // console text; meaningful only when refused
@@ -22,7 +24,8 @@ struct BoundsVerdict {
 // frame count is not the window's (render_window::renderHonoredBounds owns the
 // tolerance and its limits), or the file cannot be measured at all — an unmeasured
 // render is not a verified one. TailMode::Auto/Manual add frames by design and are
-// never judged here. `projectDir` is where a refused render is parked.
+// never judged here. `projectDir` and `request.destination` together decide where a
+// refused render is parked.
 BoundsVerdict checkRenderedBounds(const std::string& renderedPath,
                                   const std::string& projectDir,
                                   const CaptureRequest& request);
@@ -31,11 +34,12 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath,
 // never legitimately produce zero), independent of and ahead of the TailMode::None-only
 // gate above, which does not run on Auto/Manual at all.
 BoundsVerdict checkRenderedFileNotEmpty(const std::string& renderedPath,
-                                        const std::string& projectDir);
+                                        const std::string& projectDir,
+                                        CaptureDestination destination);
 
-// Where a refused render is retained -- exposed so a multi-unit caller (batch capture)
-// can name the folder once without duplicating the subfolder name `checkRenderedBounds`
-// and `checkRenderedFileNotEmpty` already use internally.
+// Where a refused Bank-destination render is retained -- exposed so a multi-unit caller
+// (batch capture, Bank-only) can name the folder once without duplicating the subfolder
+// name `checkRenderedBounds` and `checkRenderedFileNotEmpty` already use internally.
 std::string refusedRenderFolder(const std::string& projectDir);
 
 } // namespace reasampler::capture

src/shell/capture/render_in_place.cpp

@@ -2,9 +2,6 @@
 //
 // Includes reaper_plugin_functions.h WITHOUT REAPERAPI_IMPLEMENT — main.cpp is the
 // one TU that defines the API pointers; here they are extern.
-//
-// Traffic is one-way: this borrows capture's render, and capture may never borrow
-// this placement back.
 
 #include "shell/capture/render_in_place.h"
 
@@ -121,7 +118,11 @@ void RunRenderTrackInPlace(ReaSamplerSession& session) {
 
     const int trackCount = CountTracks(proj);
     const int srcIndex   = indexOfTrack(proj, trackCount, source);
-    if (srcIndex < 0) { refuse("the source track is no longer in the project"); return; }
+    if (srcIndex < 0) {
+        refuse("the source track is no longer in the project; the render landed at " +
+               res.absolutePath + " but was not placed.");
+        return;
+    }
 
     const SiblingPlacement place =
         siblingPlacement(folderDepths(proj, trackCount), srcIndex);
@@ -137,8 +138,13 @@ void RunRenderTrackInPlace(ReaSamplerSession& session) {
     InsertTrackInProject(proj, place.insertIndex, /*flags=*/0);
     MediaTrack* fresh = GetTrack(proj, place.insertIndex);
     if (!fresh) {
-        Undo_EndBlock2(nullptr, "", 0);
-        refuse("could not create the result track");
+        // InsertTrackInProject already mutated the project by this point, so the
+        // "no ext-state write -> discard" idiom does not apply here — a discard would
+        // leave the orphaned track un-undoable.
+        Undo_EndBlock2(nullptr, "ReaSampler: render in place (failed to create result track)",
+                       -1);
+        refuse("could not create the result track; the render landed at " +
+               res.absolutePath + " but was not placed.");
         return;
     }
 
@@ -151,7 +157,6 @@ void RunRenderTrackInPlace(ReaSamplerSession& session) {
     }
     SetMediaTrackInfo_Value(fresh, "I_FOLDERDEPTH",
                             static_cast<double>(place.newDepth));
-    TrackList_AdjustWindows(false);
 
     // GetTrackColor returns the colour already OR'd with 0x1000000 and 0 for "no
     // colour set", which I_CUSTOMCOLOR reads as unused — so one line clones a colour
@@ -161,6 +166,9 @@ void RunRenderTrackInPlace(ReaSamplerSession& session) {
     const std::string freshName = captureTrackName(trackName(source));
     setTrackName(fresh, freshName);
 
+    // After every attribute write, per the SDK header's manual-panel-update caveat.
+    TrackList_AdjustWindows(false);
+
     // Unsnapped and unrounded, deliberately: this placement IS the null test performed
     // automatically, so snapping it to the grid would move the audio off the position
     // it was rendered from. InsertOptions{} defaults give native length and no conform.