docs: retire the bake mono collapse and FX-GUID keying entries, file what they left behind

Both landed with accepted consequences rather than avoided ones, so COMPLETED carries the identity move and the offline-FX degrade. Two line-ceiling entries and the DAW-unverified GUID stability are now tracked.
2026-08-02 20:10:01 -04:00
parent 430faff230
commit bd7d6857e5
2 changed files with 113 additions and 52 deletions
@@ -1135,3 +1135,50 @@ exercises "the manifest parses, the layout computes, the exact-size proof fails"
 the payload boundary instead. No production module was touched — the wave adds test-tree
 files, the corpus, its README, the verification script, and one path variable in the root
 `CMakeLists.txt`.
 ### Resample-bake mono collapse — closes the `docs/TODO.md` deferral (ad-hoc)
 `prepareLanding` (`src/shell/capture/bake_landing.cpp`) now applies the shared lossless
 mono collapse to the staged buffer — via a new thin wrapper `applyMonoCollapse` in
 `src/core/capture/wav_codec.cpp` — before the hash and the channel-count read, so the
 hash, the entry, and the written file all come from one collapsed buffer. A dead-center
 (dual-mono) bake now lands as a 1-channel file exactly as a dead-center offline capture
 already does; a true-stereo bake is byte-identical to before, asserted on bytes and on
 hash.
 **The blocker this deferral originally cited has cleared.** `bake_land.cpp` was
 another team's freshly-landed remediation surface at the time; that remediation has
 since landed, which is what made taking this item this wave safe.
 **Consequences accepted, not avoided:**
 - A dead-center bake's **content identity moves** — the hash now covers the collapsed
  bytes, so a dual-mono bake will not hash-dedup against a stereo twin already in the
  bank, and its derived file name changes. This was already documented as accepted for
  the other capture paths in `src/core/capture/CLAUDE.md`; the bake path now inherits
  it rather than being an exception.
 - `BakeOutcome::channelCount` now answers 1 for a dead-center bake, which flips the
  instrument's channel-mode auto-default to Mono. Safe: the audio is identical either
  way when the source was dead-center, and the consuming site was already written
  anticipating that value.
 ### Design View FX-GUID keying for `restoreFxOffline` — closes the `docs/TODO.md` deferral (ad-hoc)
 `restoreFxOffline` (`src/shell/view/view.cpp`) now returns each parked track's per-FX
 offline state to the plugin it was captured from, keyed by the FX's own GUID
 (`TrackFX_GetFXGUID`) rather than its slot index. New pure module
 `src/core/view/fx_offline.{h,cpp}` holds the keying types (`FxKeying`: Identity/Slot),
 the per-FX snapshot/plan types, and `resolveFxRestore`, which matches each captured
 state against the chain as it stands at restore time. `view_state` gained a v2 schema
 that writes the identity array beside the v1 slot array, so an older build reading a
 v2 blob keeps the behaviour it had rather than losing every FX state.
 **Consequences accepted, not avoided:**
 - A dropped FX (one whose captured identity is no longer live in the chain at restore
  time) is left **offline**, as park left it, with its snapshot already cleared — the
  console report names the drop and the recovery.
 - The console report uses the quiet `!SHOW:` form on every path, so it never
  force-opens the console window.
 **Left open, filed to `docs/TODO.md`:** FX-GUID stability itself — whether
 `TrackFX_GetFXGUID` survives a chain reorder while parked — is unverified in the DAW
 (SWS issue #802 names a specific way it might not hold).
@@ -110,20 +110,6 @@ Forward-looking follow-ups. Deferred by decision, not oversight — each entry r
 **Done looks like.** Not stated in PLAN.md.
 ## FX-GUID keying for `restoreFxOffline` (Design View park/restore)
 **Context.** CONTEXT.md's "Open questions to resolve during build" (Design View section): the bulk of reconcile residuals shipped (`ViewModeModel::reconcile(liveGuids)` prunes orphaned snapshots on every toggle/load; folder restructure is self-healing because the tree is rebuilt each toggle; membership is intentionally kept so undo-delete preserves the tag). Two sub-items were left deferred out of that; this is the first.
 **The wart.** `restoreFxOffline` currently restores per-FX offline state by slot index. If the FX chain is reshuffled while a track is parked, restore lands on whatever plugin now occupies that slot rather than the plugin it was originally captured from.
 **Intended fix.** FX-GUID keying — key the per-FX offline snapshot entries by FX identity rather than slot index.
 **The constraint the fix MUST handle.** The keying change requires a snapshot-schema migration; CONTEXT.md names this alongside the keying change as the reason the fix was deferred rather than folded into the reconcile-residuals work.
 **Priority / risk.** Not stated in the source.
 **Done looks like.** Not stated in the source beyond the fix description above.
 ## Dormant membership entries in persisted `view_state`
 **Context.** CONTEXT.md's "Open questions to resolve during build" (Design View section), the second of the two sub-items left deferred after the reconcile-residuals ship described above.
@@ -346,6 +332,15 @@ buffer.
 shared with every other caller in `core/capture/wav_codec`; a fix must not change
 those callers' contract or add a second WAV-building code path to maintain.
 **Re-confirmed still accurate (2026-08-02), after the mono-collapse landing touched
 `wav_codec` adjacent to this site.** `applyMonoCollapse` operates on the staged bytes in
 `bake_landing.cpp`'s `prepareLanding`, upstream of and unrelated to `runBake`'s
 `std::vector<double>` copy in `instrument_bake.cpp`; `buildFloat32Wav`'s signature is
 unchanged. The wart stands exactly as described above.
 **Current blocker.** Not taken this wave because `instrument_bake.cpp` is being edited
 by a live VST3-parameter track.
 **Priority / risk.** Low / deferred. Logged at Ξ-W2-T1's review; correctness is
 unaffected, only peak memory on a large bake.
@@ -650,44 +645,6 @@ select/move the neighbour, or capture at track scope instead.
 non-isolation as an oversight and re-propose closing it against the recipe's stated
 tracks-and-range-only shape.
 ## Resample-bake landings don't apply the lossless mono collapse to a dual-mono render
 **Context (surfaced by Ψ-W2-T2, mono-collapse).** The collapse (`collapseCapturedFileToMono`
 / `core/capture/wav_codec::collapseToMono`) ships for every extension capture path —
 offline, realtime, batch, recapture — but not for `bake_land.cpp`'s landing, the
 resample bake's `prepareLanding` / `commitLanding` pair. A dead-center instrument render (the common case
 that motivated Ψ.6 in the first place) is exactly the dual-mono shape the predicate
 collapses, so an un-collapsed bake keeps paying for the second channel it doesn't need.
 **Not deferred for the reason once given.** `prepareLanding` reads the staged file into
 `prep.bytes` once, parses its layout, hashes it and derives the channel count from that
 same one buffer, and `commitLanding` writes that buffer — so collapsing it right after the
 layout parse would keep the hash, the channel count, and the written file consistent by
 construction; there is no ordering hazard here to defer around.
 **The real reason.** `bake_land.cpp` is Phase Ξ's freshly-landed surface
 (Ξ-W2-T1, the resample bake chain) and another team is actively remediating it. Landing
 a mutation there now would cross tracks mid-remediation for no urgent gain — the mono
 propagation this item would add is a size win, not a correctness one.
 **A mono capture already propagates through the bake for free**, so this item is scoped
 to the dual-mono-*render* case only: `runBake` / `instrument_bake.cpp` already renders
 however many channels the dialed sound has, and `bake_render.cpp:38` reads
 `sample.channelCount()` off that render rather than hardcoding 2 — a mono-programmed
 sound already bakes to a mono file today, with no change needed.
 **Intended fix.** Once `bake_land.cpp` is quiet, call `collapseToMono` on `prep.bytes` in
 `prepareLanding` right after the layout parse and before the hash, matching the
 offline/realtime insertion point (post-parse, pre-identity-read).
 **Priority / risk.** Low — a size optimization on an already-correct path, not a
 precision-invariant gap; the bake's dual-mono case still lands as a valid (if larger)
 stereo file today.
 **Done looks like.** A dead-center instrument bake lands as a 1-channel file with
 `Sample::channelCount` matching, the same way an offline dead-center capture does; a
 true-stereo bake is byte-identical to today's output.
 ## A 0-byte render can still pass every gate under Auto/Manual tail (closed)
 **Context (surfaced by Ψ-W3 review).** `OfflineRenderBackend::capture`'s exists-check
@@ -916,3 +873,60 @@ doc-keeper edit.
 **Done looks like.** The enumeration distinguishes "in the project's state" from "on disk
 in the `.rpp`", and does not gain a second home for the distinction.
 ## `view_mode_model.cpp` is over the ~600-line structural bar, and `view.cpp` is close behind
 **Context (surfaced by the FX-GUID keying track).** Root `CLAUDE.md`'s structural
 heuristics put an ~600-line ceiling on any one file, with a documented responsibility
 seam as the required method for splitting it, not an arbitrary bisection.
 `src/core/view/view_mode_model.cpp` measures **815 lines** (verified this pass),
 up from 715 before the FX-GUID keying track's v2 schema addition made it worse.
 **The named seam.** The JSON codec — `serialize()`/`deserialize()` — wants its own
 `view_state_codec` TU in `src/core/view/`.
 **Why it was deferred, and this reasoning should survive.** `serialize()` is a
 `ViewModeModel` member and `deserialize()` a static factory (confirmed:
 `std::string ViewModeModel::serialize() const` and
 `std::optional<ViewModeModel> ViewModeModel::deserialize(const std::string&)`), both
 reaching private state — so extraction needs either a friend declaration or a new
 public accessor surface. Doing that in the same commit that changed the byte format
 the golden test literals pin would roll a format change and a codec extraction
 together, which is the riskier order.
 **Also over the bar, blocked differently.** `src/shell/view/view.cpp` measures
 **642 lines** (verified this pass). Its seam is blocked not by a private-state/friend
 question but by a build file another team owns: `src/shell/view/` has no
 `CMakeLists.txt` of its own today.
 **Priority / risk.** Not stated.
 **Done looks like.** `view_mode_model.cpp`'s JSON codec is extracted into its own
 `view_state_codec` TU (with the friend/accessor question resolved deliberately, not
 sidestepped), dropping the file under the ~600-line ceiling; `view.cpp`'s own path is
 unblocked once the build-file ownership question is resolved.
 ## FX-GUID stability for `restoreFxOffline` is unverified in the DAW
 **Context.** The Design View park/restore FX keying (`restoreFxOffline`,
 `src/shell/view/view.cpp`) rests on `TrackFX_GetFXGUID` returning an identity that
 survives a chain reorder while a track is parked. SWS issue #802 reports that after
 `SNM_MoveOrRemoveTrackFX` reorders a chain, the FXID lines do not follow the plugin
 (`SNM_PreObjectState()` → `RemoveAllIds()`) — if that still holds, an SWS-driven
 reorder while parked produces wrong-plugin restores or mass drops, which is the exact
 operation this keying targets.
 **What must be checked.** Native drag-reorder, an SWS move, save/reload, and two live
 instances of the same plugin.
 **Already flagged in code — this entry is the tracked home, not a restatement.**
 There is a `[verify — DAW]` marker at `fxGuidString` in `src/shell/view/view.cpp` and
 a note in `src/shell/view/CLAUDE.md`'s Gotchas; point at them rather than restating
 them in full.
 **Priority / risk.** Not stated.
 **Done looks like.** Native reorder, SWS reorder, save/reload, and a
 two-instance-of-the-same-plugin case are each observed in a live REAPER session, and
 either the identity is confirmed to survive all four, or a degradation is found and
 the keying is amended.