diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index dc39592..2b9e343 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -1750,12 +1750,16 @@ bool isFixedLaneTrack(MediaTrack* tr) { // Enumerates the live project's track + item GUIDs. Fills `allGuids` (the full live set, // baseline input) and, for each item, records whether it sits on a manual lane so a // newly-detected item can be exempted from auto-tag without a second project walk. +// `trackItemGuids` additionally maps each track GUID to the item GUIDs it carries, so a +// newly-detected item's PRE-EXISTING siblings can be resolved (the adoption / strand +// guard) without a second project walk. // // Manual-lane classification uses the single pure predicate isOnManualLane(isFixedLaneTrack, // laneName) from lane_keys — the same predicate the apply path consults — so the exemption // rule is defined in exactly one place and is unit-tested there. void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, - std::map& itemOnManualLane) { + std::map& itemOnManualLane, + std::map>& trackItemGuids) { const int trackCount = CountTracks(proj); for (int t = 0; t < trackCount; ++t) { MediaTrack* tr = GetTrack(proj, t); @@ -1767,6 +1771,7 @@ void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, // track-level attribute and is the same for every item on the track. const bool fixedLane = isFixedLaneTrack(tr); + std::vector& itemsOnTrack = trackItemGuids[tg]; const int itemCount = CountTrackMediaItems(tr); for (int i = 0; i < itemCount; ++i) { MediaItem* it = GetTrackMediaItem(tr, i); @@ -1779,6 +1784,7 @@ void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, // false immediately for non-fixed-lane tracks regardless of name). const std::string ln = fixedLane ? itemLaneName(tr, it) : std::string{}; itemOnManualLane[ig] = isOnManualLane(fixedLane, ln); + itemsOnTrack.push_back(ig); } } } @@ -1818,11 +1824,52 @@ bool detectNewContent() { std::set live; std::map itemOnManualLane; - enumerateLiveGuids(proj, live, itemOnManualLane); + std::map> trackItemGuids; + enumerateLiveGuids(proj, live, itemOnManualLane, trackItemGuids); const std::vector added = g_panel.contentBaseline.observe(live); if (added.empty()) return false; // first poll after open, or nothing new this tick + ViewModeModel& model = g_panel.session->view(); + + // Which of `added` are items (the manual-lane map keys every item; track GUIDs never + // appear there). Used below to exclude sibling new items from a track's PRE-EXISTING + // mode set — a drop plus its own new siblings must not count each other as prior. + const std::set newItemGuids = [&] { + std::set s; + for (const std::string& g : added) + if (itemOnManualLane.count(g)) s.insert(g); + return s; + }(); + + // Item guid -> its track guid (reverse of trackItemGuids), so a new item's siblings + // are found in one lookup. + std::map trackOfItem; + for (const auto& [trackGuid, items] : trackItemGuids) + for (const std::string& ig : items) trackOfItem[ig] = trackGuid; + + // The distinct modes the PRE-EXISTING (not-new-this-tick) MANAGED-ELIGIBLE items on + // `trackGuid` resolve to. Untagged siblings resolve to Arrange (leafBelongsToMode's + // default); new siblings are excluded; manual-lane siblings are EXEMPT — exactly as + // planLaneMinting ignores them when computing a track's own-item mode span, so the + // adoption guard's view of the track matches the split decision's. Drives the adoption + // / strand guard in autoTagNewContent. + const auto preExistingTrackModes = + [&](const std::string& trackGuid) -> std::set { + std::set modes; + auto it = trackItemGuids.find(trackGuid); + if (it == trackItemGuids.end()) return modes; + for (const std::string& sib : it->second) { + if (newItemGuids.count(sib)) continue; // a sibling added THIS tick — not prior + auto ml = itemOnManualLane.find(sib); + if (ml != itemOnManualLane.end() && ml->second) continue; // manual lane — exempt + const std::set m = model.membership().modesOf(sib); + if (m.empty()) modes.insert(kArrangeModeId); // untagged ⇒ Arrange default + else modes.insert(m.begin(), m.end()); + } + return modes; + }; + // Split the new GUIDs into tracks vs items so the pure decision can apply the // manual-lane exemption to items only. A GUID present in the item-lane map is an // item; otherwise it is a track (track GUIDs never appear in that map). @@ -1833,11 +1880,13 @@ bool detectNewContent() { if (it == itemOnManualLane.end()) { newTracks.push_back(g); // a track GUID } else { - newItems.push_back(NewItem{g, it->second}); // an item; carries its exemption + NewItem ni{g, it->second, {}}; + auto tk = trackOfItem.find(g); + if (tk != trackOfItem.end()) ni.trackModes = preExistingTrackModes(tk->second); + newItems.push_back(std::move(ni)); // an item; carries exemption + track modes } } - ViewModeModel& model = g_panel.session->view(); const std::vector tags = autoTagNewContent(newTracks, newItems, model.activeModeId()); for (const AutoTag& tag : tags) diff --git a/src/view_mode_model.cpp b/src/view_mode_model.cpp index 7528e1a..9b6af0b 100644 --- a/src/view_mode_model.cpp +++ b/src/view_mode_model.cpp @@ -153,7 +153,16 @@ std::vector autoTagNewContent(const std::vector& newTrackG for (const auto& item : newItems) { if (item.guid.empty()) continue; if (item.onManualLane) continue; // manual-lane content is off-limits to auto-tag - tags.push_back(AutoTag{item.guid, activeMode}); + + // ADOPTION (strand guard): a new item on a track whose PRE-EXISTING content + // resolves to exactly one mode adopts THAT mode, so a drop onto a track already + // showing content never pushes it multi-mode and never triggers a lane split that + // would silence the pre-existing, previously-visible items. A track with no prior + // content (empty trackModes) or one already carrying a deliberate multi-mode split + // (>1) falls back to the active-mode rule. + const std::string& target = + item.trackModes.size() == 1 ? *item.trackModes.begin() : activeMode; + tags.push_back(AutoTag{item.guid, target}); } return tags; } diff --git a/src/view_mode_model.h b/src/view_mode_model.h index 42445f5..bcd0765 100644 --- a/src/view_mode_model.h +++ b/src/view_mode_model.h @@ -493,11 +493,35 @@ TrackPlan makeRestorePlan(const std::string& guid, const TrackSnapshot& snap); // such an item `onManualLane = true` (it knows the item's lane and consults the // ownership index); the decision then emits NO tag for it. New tracks and new items on // managed/no lane follow the active-mode rule. +// +// -- Pre-existing-content adoption (strand fix) ------------------------------- +// +// A new item dropped onto a track that ALREADY carries currently-visible content must +// not silently push that track into a different mode. If the pre-existing content +// resolves to ONE mode and the new item were blindly tagged to the (different) ACTIVE +// mode, the track would become multi-mode, planLaneMinting would split it, and the +// toggle would silence whichever lane the active mode does not own — stranding the +// pre-existing, previously-visible items on a C_LANEPLAYS=0 lane with no user intent. +// +// The rule: a new item ADOPTS the single mode of the pre-existing content already on its +// track. Only when the track carries no pre-existing managed-eligible content (an empty +// or brand-new track), or when that content already spans multiple modes (an existing +// deliberate split, which the new item joins under the active mode), does the new item +// fall back to the active-mode rule. Deliberate two-take splits are unaffected: those go +// through the explicit item mode-move actions (planItemRetag), never auto-tag. +// The shell reports each new item's track pre-existing-content modes in `trackModes`. -// One new item the shell detected this poll. Its lane disposition decides exemption. +// One new item the shell detected this poll. Its lane disposition decides exemption; its +// track's pre-existing content modes decide adoption (see above). struct NewItem { std::string guid; bool onManualLane = false; // true ⇒ EXEMPT from auto-tag (hand-managed lane) + // The distinct modes the PRE-EXISTING (not-new-this-tick) managed-eligible content on + // this item's track resolves to. Empty ⇒ the item's track carried no prior content, so + // the item takes the active mode. Exactly one ⇒ ADOPT that mode (the strand guard). + // More than one ⇒ the track is already a deliberate split; the item takes the active + // mode. The shell fills this by resolving each pre-existing item's mode from membership. + std::set trackModes; }; // One membership write the auto-tag decision produced: tag `guid` into `modeId`. The @@ -511,10 +535,11 @@ struct AutoTag { // The pure auto-tag decision: given the new track GUIDs and new items detected this // poll plus the active mode, produce the membership writes. Every new track is tagged -// to `activeMode`; every new item is tagged to `activeMode` UNLESS it landed on a -// manual lane (exempt). An empty `activeMode` yields no tags (nothing to tag into). -// Empty GUIDs are skipped. The result is a plan the shell applies; this function -// mutates nothing. +// to `activeMode`. Every new item is tagged UNLESS it landed on a manual lane (exempt); +// its target mode is the single mode of its track's pre-existing content (adoption — the +// strand guard) when that content resolves to exactly one mode, otherwise `activeMode`. +// An empty `activeMode` yields no tags (nothing to tag into). Empty GUIDs are skipped. +// The result is a plan the shell applies; this function mutates nothing. std::vector autoTagNewContent(const std::vector& newTrackGuids, const std::vector& newItems, const std::string& activeMode); diff --git a/tests/test_view_mode_model.cpp b/tests/test_view_mode_model.cpp index a659b0e..94ce056 100644 --- a/tests/test_view_mode_model.cpp +++ b/tests/test_view_mode_model.cpp @@ -1072,6 +1072,150 @@ static void testAutoTagDecision() { CHECK(vm.leafBelongsToMode("{PREEXISTING}", kArrangeModeId)); // ⇒ Arrange CHECK(!vm.leafBelongsToMode("{PREEXISTING}", kDesignModeId)); } + + // ADOPTION / STRAND GUARD: a new item dropped onto a track whose pre-existing content + // resolves to a SINGLE mode adopts THAT mode, NOT the (different) active mode — so the + // track never becomes multi-mode and no silencing lane split is triggered. This is the + // exact drop-onto-tagged-track repro at the auto-tag boundary: active mode = Design, + // the track already carries Arrange content ⇒ the drop is tagged Arrange (adopted), + // keeping the pre-existing Arrange items on the visible/playing surface. + { + NewItem dropped{"{DROP}", /*onManualLane=*/false, /*trackModes=*/{kArrangeModeId}}; + auto tags = autoTagNewContent({}, {dropped}, kDesignModeId); + CHECK(tags.size() == 1); + CHECK(hasTag(tags, "{DROP}", kArrangeModeId)); // adopted, NOT Design + CHECK(!hasTag(tags, "{DROP}", kDesignModeId)); + } + + // Adoption is symmetric: pre-existing Design content + active Arrange ⇒ adopt Design. + { + NewItem dropped{"{DROP}", false, {kDesignModeId}}; + auto tags = autoTagNewContent({}, {dropped}, kArrangeModeId); + CHECK(tags.size() == 1); + CHECK(hasTag(tags, "{DROP}", kDesignModeId)); + } + + // No pre-existing content (empty trackModes — a brand-new/empty track) ⇒ the item + // takes the ACTIVE mode (unchanged behaviour; adoption only fires with prior content). + { + NewItem dropped{"{DROP}", false, /*trackModes=*/{}}; + auto tags = autoTagNewContent({}, {dropped}, kDesignModeId); + CHECK(tags.size() == 1); + CHECK(hasTag(tags, "{DROP}", kDesignModeId)); + } + + // Pre-existing content ALREADY spans >1 mode (a deliberate split) ⇒ the new item + // takes the ACTIVE mode and joins the active lane; adoption does not fire (no single + // mode to adopt), and the existing split — with both lanes present — cannot strand. + { + NewItem dropped{"{DROP}", false, {kArrangeModeId, kDesignModeId}}; + auto tags = autoTagNewContent({}, {dropped}, kDesignModeId); + CHECK(tags.size() == 1); + CHECK(hasTag(tags, "{DROP}", kDesignModeId)); // active mode, not adopted + } + + // Adoption composes with the manual-lane exemption: a manual-lane item is still exempt + // regardless of its track's pre-existing modes (no tag emitted at all). + { + NewItem manual{"{MANUAL}", /*onManualLane=*/true, {kArrangeModeId}}; + auto tags = autoTagNewContent({}, {manual}, kDesignModeId); + CHECK(tags.empty()); + } + + // A new TRACK still takes the active mode — adoption is an ITEM rule only (a track has + // no "pre-existing content on the same track" notion). + { + auto tags = autoTagNewContent({"{NT}"}, {}, kDesignModeId); + CHECK(tags.size() == 1); + CHECK(hasTag(tags, "{NT}", kDesignModeId)); + } +} + +// -- Drop-onto-tagged-track STRAND repro (end-to-end at the pure-model level) -- +// +// The reported bug: a Design-tagged track carries pre-existing (untagged ⇒ Arrange) +// items; while Design is the active mode the user drops a capture onto the track. The +// old auto-tag rule tagged the drop into the ACTIVE mode (Design) even though the +// track's own content was Arrange; the track went multi-mode; planLaneMinting split it; +// planToggle drove the Arrange lane C_LANEPLAYS=0 — stranding the pre-existing, +// previously-visible items on a silenced lane with no user intent. +// +// This test drives the WHOLE decision chain the shell runs on a drop tick — detect the +// new item, resolve its track's pre-existing modes, autoTagNewContent, apply the tag, +// then planLaneMinting + planToggle — and asserts the invariant directly: NO lane +// holding a pre-existing item ends silenced under the active mode. +static void testDropOntoTaggedTrackDoesNotStrand() { + const std::string track = "{T}"; + const std::string preA = "{arr-pre-1}"; // pre-existing untagged ⇒ Arrange + const std::string preB = "{arr-pre-2}"; // pre-existing untagged ⇒ Arrange + const std::string drop = "{drop}"; // the capture just dropped onto the track + + ViewModeModel vm; + vm.membership().tag(track, kDesignModeId); // the TRACK is tagged Design (leaf tag) + vm.setActiveMode(kDesignModeId); // user is viewing Design when they drop + // Pre-existing items are UNTAGGED (they resolve to Arrange) — never auto-tagged (they + // predate the baseline). Leave them absent from the membership index. + + // Shell resolves the drop's track pre-existing modes: both siblings are untagged ⇒ + // {arrange}. Exactly one mode ⇒ the adoption guard fires. + NewItem dropped{drop, /*onManualLane=*/false, /*trackModes=*/{kArrangeModeId}}; + const std::vector tags = + autoTagNewContent({}, {dropped}, vm.activeModeId()); + for (const AutoTag& t : tags) vm.membership().tag(t.guid, t.modeId); + + // FIX ASSERTION 1: the drop adopted Arrange, so the track's OWN items are all one mode. + CHECK(vm.membership().modesOf(drop) == std::set{kArrangeModeId}); + + // Run the lane-minting decision exactly as the shell does after the tag. + FolderTree tree; + tree.nodes.push_back(FolderNode{track, "", false}); + std::vector tracks{ + LaneTrack{track, { + LaneItem{preA, kArrangeModeId, false}, + LaneItem{preB, kArrangeModeId, false}, + LaneItem{drop, kArrangeModeId, false}, // adopted ⇒ Arrange + }}, + }; + const LaneMintPlan plan = planLaneMinting(vm, tree, tracks); + + // FIX ASSERTION 2: single-mode track ⇒ NO split at all. Nothing is minted, nothing is + // reassigned, so the pre-existing items stay exactly where they were and visible. + CHECK(plan.empty()); + + // FIX ASSERTION 3 (the invariant, stated positively): apply whatever lanes the plan + // WOULD mint into the ownership index, then toggle to the active mode and assert NO + // lane carrying a pre-existing item is silenced. With no split the ownership index is + // empty and the toggle emits no silencing op — the pre-existing items cannot be + // stranded. (Belt-and-braces: the same assertion would catch a regression that + // re-introduced the split.) + for (const LaneMint& m : plan.mints) + vm.lanes().setManaged(m.trackGuid, m.laneKey, m.modeId); + const TogglePlan toggle = vm.planToggle(tree, vm.activeModeId()); + // The Arrange lane (if it existed) would be laneNameForMode(kArrangeModeId). Under a + // correct fix it never exists; assert it is not driven to silent either way. + CHECK(lanePlaysFor(toggle, track, laneNameForMode(kArrangeModeId)) != kLaneSilent); + + // CONTRAST — the OLD (buggy) behaviour, reproduced by forcing the active-mode tag: if + // the drop had been tagged Design (active) instead of adopting Arrange, the track WOULD + // split and the Arrange lane WOULD be silenced under Design. This proves the test can + // disprove the bug — it is not tautological. + ViewModeModel buggy; + buggy.membership().tag(track, kDesignModeId); + buggy.setActiveMode(kDesignModeId); + buggy.membership().tag(drop, kDesignModeId); // the old active-mode tag + std::vector buggyTracks{ + LaneTrack{track, { + LaneItem{preA, kArrangeModeId, false}, + LaneItem{preB, kArrangeModeId, false}, + LaneItem{drop, kDesignModeId, false}, // Design (active) ⇒ 2nd mode + }}, + }; + const LaneMintPlan buggyPlan = planLaneMinting(buggy, tree, buggyTracks); + CHECK(!buggyPlan.empty()); // the old path DID split + for (const LaneMint& m : buggyPlan.mints) + buggy.lanes().setManaged(m.trackGuid, m.laneKey, m.modeId); + const TogglePlan buggyToggle = buggy.planToggle(tree, kDesignModeId); + CHECK(lanePlaysFor(buggyToggle, track, laneNameForMode(kArrangeModeId)) == kLaneSilent); } // -- D2 W3-B item-level mode-move decision ----------------------------------- @@ -1666,6 +1810,7 @@ int main() { testLaneOwnershipLastWriterWins(); testManagedOnlyPlannerAndQuery(); testAutoTagDecision(); + testDropOntoTaggedTrackDoesNotStrand(); testPlanItemRetag(); testReconcileUnregisteredModeGuardDecision(); testLaneMintingSingleModeNoSplit();