From 02b6350dc97e0a627b90701b85b5df5f1f8a7879 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 19:26:06 -0400 Subject: [PATCH] fix(bank_panel): drive new-content baseline re-arm from persist's load signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The detector re-armed its GuidBaseline on a pointer compare (proj != lastProject), a weaker signal than persist's GUID-primary identity. On a load onto a recycled ReaProject* the baseline never reset, so the just-loaded project's pre-existing tracks diffed against the previous project and were mass-tagged into the active mode — opening a Design-saved project mis-tagged its Arrange tracks. Re-arm now rides persist's authoritative load signal via bankPanelNotifyProjectLoaded(). --- src/bank_panel.cpp | 58 +++++++++++++++++++++++++++++----------- src/bank_panel.h | 11 ++++++++ src/main.cpp | 10 ++++++- tests/test_guid_diff.cpp | 57 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 8fbaadf..3203d70 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -227,15 +227,26 @@ struct PanelState { // // Each timer tick diffs the live track+item GUID set against the previous tick to // auto-tag content created SINCE the last tick into the then-active mode. The - // baseline carries the first-poll-after-open guard so pre-existing content is never - // mass-tagged (it stays Arrange). `lastProject` detects a project switch so the - // baseline re-arms per project (a switch never diffs across two projects). Both live - // for the extension's lifetime alongside the session, independent of panel open/close - // — detection must run whether or not the dock is visible (content is created in the - // arrange, not the panel). + // baseline carries the first-poll-after-open guard (GuidBaseline self-arms on its + // first observe()) so pre-existing content is never mass-tagged (it stays Arrange). + // + // Project-load re-arm is driven by persist's AUTHORITATIVE load lifecycle, NOT by a + // pointer compare here. main.cpp calls bankPanelNotifyProjectLoaded() on the exact + // tick persist restores a project's membership + active mode (the same tick it + // reapplies the active mode); that sets reloadPending so the NEXT detect tick this + // same tick re-baselines against the fully-loaded set and reports nothing new. This + // replaces the former `proj != lastProject` re-arm, which used a WEAKER signal than + // persist (pointer-only vs persist's GUID-primary identity) and so missed a load onto + // a RECYCLED ReaProject* address — the just-loaded project's pre-existing tracks then + // diffed against the previous project's stale baseline and were mass-tagged into the + // active mode (the reload-mis-tag bug). Coordinating with persist's signal makes the + // two identity checks agree by construction. + // + // Lives for the extension's lifetime alongside the session, independent of panel + // open/close — detection must run whether or not the dock is visible (content is + // created in the arrange, not the panel). GuidBaseline contentBaseline; - ReaProject* lastProject = nullptr; - bool sawProject = false; // false until the first detect tick sees a project + bool reloadPending = false; // set by bankPanelNotifyProjectLoaded; drained next detect tick }; PanelState g_panel; @@ -714,14 +725,19 @@ void detectNewContent() { ReaProject* proj = EnumProjects(-1, nullptr, 0); - // Project switch (or first ever tick) re-arms the first-poll guard so we never diff - // across two projects. GUID-address recycling is bounded here: a missed reset can at - // worst re-baseline against the wrong project for one tick; the identity-of-record - // (persist's minted project GUID) governs the bank/model reload, not this detector. - if (!g_panel.sawProject || proj != g_panel.lastProject) { + // A project (re)load re-arms the first-poll guard so we never diff across two + // projects. The signal is persist's — main.cpp calls bankPanelNotifyProjectLoaded() + // on the tick persist restores the project's membership + active mode, which sets + // reloadPending. Draining it here re-baselines against the fully-loaded set (that + // same tick's reapply-active-mode enumerated those tracks, so they are present), + // and the observe() below returns nothing new — pre-existing untagged tracks stay + // Arrange. GuidBaseline self-arms on its first observe() for the very first tick, so + // no separate first-tick handling is needed here. Using persist's GUID-primary load + // signal (not a local pointer compare) is what fixes the reload-mis-tag: the two + // identity checks can no longer diverge on a recycled ReaProject* address. + if (g_panel.reloadPending) { g_panel.contentBaseline.reset(); - g_panel.lastProject = proj; - g_panel.sawProject = true; + g_panel.reloadPending = false; } std::set live; @@ -1141,6 +1157,18 @@ std::vector bankPanelSelectedSampleIds() { return ids; } +void bankPanelNotifyProjectLoaded() { + // Persist restored a project's membership + active mode this tick (main.cpp calls + // this from the same consumeLoadSignal() branch that reapplies the active mode). + // Arm the new-content detector to re-baseline on its next tick so the just-loaded + // project's pre-existing content is treated as the baseline (nothing new) rather + // than diffed against the previous project and mass-tagged into the active mode. + // A flag (not an inline reset) because detectNewContent owns the baseline and runs + // later in the SAME OnTimer tick — it drains this and re-baselines against the live + // set in one place, keeping the reset and the observe() adjacent and ordered. + g_panel.reloadPending = true; +} + void bankPanelRefresh() { // New-content auto-tag detection runs EVERY tick regardless of panel open/close: // tracks/items are created in the arrange view, not the panel, so detection must diff --git a/src/bank_panel.h b/src/bank_panel.h index 6e38528..bec3670 100644 --- a/src/bank_panel.h +++ b/src/bank_panel.h @@ -50,6 +50,17 @@ std::vector bankPanelSelectedSampleIds(); // reflected without the panel diffing the bank itself. void bankPanelRefresh(); +// Notifies the panel that persist just (re)loaded a project's view model (membership + +// active mode). main.cpp calls this on the exact tick it drains persist's load signal +// and reapplies the active mode. It re-arms the new-content detector so the just-loaded +// project's PRE-EXISTING content is taken as the baseline (reported as nothing new), +// never diffed against the previously-open project and mass-tagged into the active mode. +// This coordinates the detector's project-identity signal with persist's authoritative +// (GUID-primary) one — the two can no longer diverge on a recycled ReaProject* address, +// which is what caused a project opened in Design to mis-tag its Arrange tracks. READ/ +// arm of panel state only; no project or bank mutation. +void bankPanelNotifyProjectLoaded(); + // The panel's current tail-mode setting (mode + Manual length), read by the plain // CAPTURE_ITEM / CAPTURE_TRACK actions when building a CaptureRequest so a capture // applies whatever the panel toggle is set to. Default None (exact bounds) — a diff --git a/src/main.cpp b/src/main.cpp index 5763147..f19d99c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -216,8 +216,16 @@ static void OnTimer() // project saved in Design mode parks the Arrange tracks automatically, no manual // toggle. Fires exactly once per load (consumeLoadSignal clears it); idle ticks // skip it. proj = nullptr -> REAPER's active project (the one poll just loaded). - if (g_session.consumeLoadSignal()) + // + // The SAME signal re-arms the bank panel's new-content detector: a load must + // re-baseline the detector against the just-loaded project's content so its + // pre-existing tracks are never mis-detected as "new" and mass-tagged into the + // active mode (the reload-mis-tag bug). Notify BEFORE the reapply so the detector's + // re-arm and the model restore ride the one authoritative load event. + if (g_session.consumeLoadSignal()) { + reasampler::bankPanelNotifyProjectLoaded(); reasampler::applyMode(g_session.view(), g_session.view().activeModeId(), nullptr); + } // Reflect a live bank change (capture / project load) in the docked grid. // Cheap when the bank is unchanged (a fingerprint compare); repaints only on diff --git a/tests/test_guid_diff.cpp b/tests/test_guid_diff.cpp index 7c329e3..08febad 100644 --- a/tests/test_guid_diff.cpp +++ b/tests/test_guid_diff.cpp @@ -124,6 +124,61 @@ static void testResetReBaselines() { CHECK(p2new.size() == 1 && has(p2new, "{W}")); } +// -- 6. Reload-mis-tag regression: a project LOAD must re-baseline before the first +// post-load observe, so the newly-loaded project's PRE-EXISTING content is never +// reported as new. This locks the exact failure behind the reload-mis-tag bug: +// the detector used to re-arm on a `proj != lastProject` pointer compare, which a +// recycled ReaProject* address defeats; the previous project's stale baseline then +// reported the whole just-loaded project as new content and it got mass-tagged into +// the active mode. The fix routes the re-arm through persist's authoritative load +// signal (bankPanelNotifyProjectLoaded -> reset()), modeled here as: on a load, +// reset() runs BEFORE the first observe of the new project's set. +// +// The seam under test is GuidBaseline; the shell wiring (main.cpp notify -> +// bank_panel reset()) is DAW-verified, but the load-then-observe DECISION lives +// here and is what the bug got wrong. +static void testReloadReBaselinesBeforeFirstObserve() { + // Project A is open and settled: its content is the baseline, steady state reports + // nothing new. This is the "extension already running against project A" precondition + // the bug needs (a NON-empty stale baseline to mis-diff the next project against). + GuidBaseline b; + b.observe({"{A1}", "{A2}"}); // A baseline (first-poll guard) + CHECK(b.observe({"{A1}", "{A2}"}).empty()); // steady: nothing new + CHECK(b.primed()); + + // Daniel opens project B (saved in Design). B's pre-existing tracks are an ENTIRELY + // different GUID set from A. persist raises its load signal; the fix calls reset() + // (via bankPanelNotifyProjectLoaded) BEFORE the first post-load observe. + b.reset(); + auto afterLoad = b.observe({"{B1}", "{B2}", "{B3}"}); + // The load must tag NOTHING: B's pre-existing content is the baseline, not "new". + // Untagged/Arrange leaves stay Arrange; nothing is mass-tagged into Design. + CHECK(afterLoad.empty()); + + // And genuine post-load creation in B is still detected (the fix must not deafen the + // detector — only suppress the pre-existing set at the load boundary). + auto createdInB = b.observe({"{B1}", "{B2}", "{B3}", "{B4}"}); + CHECK(createdInB.size() == 1 && has(createdInB, "{B4}")); +} + +// -- 6b. Negative control: WITHOUT the load re-baseline (the old pointer-miss path where +// reset() never fired), the just-loaded project's pre-existing content IS reported +// as new — i.e. it would be mass-tagged. This proves the assertion in test 6 is +// load-bearing (the reset() is what prevents the mis-tag), not self-affirming. +static void testMissingReBaselineWouldMisTag() { + GuidBaseline b; + b.observe({"{A1}", "{A2}"}); // A baseline + b.observe({"{A1}", "{A2}"}); // settled against A + + // Simulate the BUG: no reset() on the load (the pointer compare missed a recycled + // ReaProject*). The next observe diffs B's set against A's stale baseline. + auto misdetected = b.observe({"{B1}", "{B2}", "{B3}"}); + // Every one of B's pre-existing tracks looks "new" — exactly the mass-tag that + // parked the Arrange tracks into Design on open. This is the failure the fix removes. + CHECK(misdetected.size() == 3); + CHECK(has(misdetected, "{B1}") && has(misdetected, "{B2}") && has(misdetected, "{B3}")); +} + int main() { testNewGuidsDifference(); testNewGuidsIgnoresEmpty(); @@ -131,6 +186,8 @@ int main() { testBaselineIncremental(); testBaselineDeletionReDetect(); testResetReBaselines(); + testReloadReBaselinesBeforeFirstObserve(); + testMissingReBaselineWouldMisTag(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0;