From bd7d6857e58f75120e4fbcf2dce4139cbfc3638c Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 20:10:01 -0400 Subject: [PATCH] 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. --- docs/COMPLETED.md | 47 ++++++++++++++++++ docs/TODO.md | 118 ++++++++++++++++++++++++++-------------------- 2 files changed, 113 insertions(+), 52 deletions(-) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 80e52e3..5a5e1e5 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -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). diff --git a/docs/TODO.md b/docs/TODO.md index 4a383cb..ea8084a 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -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` 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::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.