fix(studio): make a multi-selection work and survive

[miguel-heygen]

What

Selecting several elements on the canvas works and stays selected: shift-click, marquee, timeline sync, and a URL that carries the whole selection.

Why

Multi-selection was broken in several independent ways. Shift-click added whichever element was hovered last rather than the one under the pointer. A marquee kept only the first element it caught. A group erased itself when the timeline synced back. Dropping a group deselected it. A marquee could not start outside the composition frame, so elements off-canvas were hard to reach. And a link to a bug hit with several elements selected only reproduced one of them, so reports read as "works for me".

How

The timeline is the source of truth for selection, and the canvas announces into it rather than holding a competing copy. Members are published only when there is more than one, since preserving a set that does not contain the id empties it. Losing one member re-resolves the group instead of clearing it. Snapping waits for the gesture to travel before it engages, so it cannot move a selection you have not dragged yet. The hash carries the rest of the selection as selGroup and reopens it; members whose element is gone are dropped rather than failing the others.

Two gesture-geometry fixes come first, because the rest rests on them: the selection box is sized by the transform the element actually paints under, and a drag maps screen movement through the element's real ancestor transforms rather than assuming canvas zoom.

The frame handler moves to its own module on the way past. It had grown a snap block and a trace block inside a function already juggling four gesture kinds, past both the complexity and file-size gates.

Test plan

• 2203 studio editor, hook and util tests
• Marquee three elements from outside the canvas, drop them, and the selection survives; copy the hash, open it fresh, the same three come back

---

Part of a stack re-cutting #3077, which stays open as the reference for the whole tree. Preview fixes land first, then the rich-text feature.

[james-russo-rames-d-jusso]

Reviewed at 92c812131.

Right shape for the class of bug this closes: timeline as the source of truth for selection, canvas announces into it, members published only when >1, gestures gated on real ancestor transforms, URL carrying the whole group. Every top-line PR-body promise is supported by concrete code:

• Timeline is the source of truth / canvas announces — announceTimelineSelection is the single canvas→timeline write, called at every mutation site (useDomSelection.ts:175, 230, 234, 468, 550). Old inline setSelectedTimelineElementId(...) + findMatchingTimelineElementId(...) call sites are gone.
• Members published only when >1 — domSelectionTimelineMirror.ts:58 — if (group.length > 1) setTimelineSelectionSet(new Set(members));. Docstring at :12-23 names why: "anchoring with preserveSet on an id the set does not yet contain empties the set outright."
• Losing one member re-resolves the group — useDomEditPreviewSync.ts:87-95 and useDomSelection.ts:411-421 both branch on group.length > 1 and call refreshDomEditGroupSelectionsFromPreview(group) before falling through to applyDomSelection(null).
• Selection box sized by actual paint transform — domEditOverlayGeometry.ts:142-161 walks for (let node = element; node; node = node.parentElement) and left-multiplies each ancestor's getComputedStyle(node).transform; test at :182-198 proves the compose (parent scale(1.2) → oriented rect width 240 for a local-200 element).
• Drag maps screen movement through real ancestor transforms — manualOffsetDrag.ts:242-310 runs three probe writes and reads BCR centers with the iframe-scale walk at getRectCenter (:159-187); mirror-parent test at manualOffsetDrag.test.ts:100-124.
• URL selGroup round-trips — studioUrlState.ts:149-151 serialize, :74-80 parse; test at studioUrlState.test.ts:160 covers the two-id case; members dropped rather than failing the others per useStudioUrlState.ts:152-161.

The debug-helper consolidation (studioDebug.ts factory + reloadDebug/selectDebug/dragDebug sharing it) preserves the lazy-thunk shape from #3137 R1 — studioDebug.ts:9-27 accepts Record<string, unknown> | (() => Record<string, unknown>) and evaluates the thunk only after the enabled check. The R1 fix I just asked for survives the refactor.

A few concerns worth surfacing before landing.

Concerns

Single-canvas-click after multi-select (or after a timeline-select) empties the timeline's selectedElementIds and the sync loop then wipes the canvas. Trace, using the store you inherit from player/store/playerStore.ts:524-547 (which this PR doesn't modify but now composes differently against):

1. User marquee-selects [a, b]. applyMarqueeSelection([a,b], false) → announceTimelineSelection([a,b], a) → setTimelineSelectionSet(new Set([a,b])) then setSelectedTimelineElementId("a", {preserveSet:true}). Store: id=a, ids={a,b}.
2. useTimelineSelectionPreviewSync effect fires with selectedIds=["a","b"], selectedKey="a\0b", sets lastSyncedSelectedKeyRef.current = "a\0b".
3. User single-clicks canvas on d. applyDomSelection(d) (non-additive, no preserveGroup) → nextGroup=[d], nextSelection=d (useDomSelection.ts:186-199). Refs + state updated. announceTimelineSelection([d], d) fires — group.length===1, so setTimelineSelectionSet is NOT called; only setSelectedTimelineElementId("d", {preserveSet:true}).
4. Store logic at playerStore.ts:526: preserveSet = Boolean(options?.preserveSet && id && s.selectedElementIds.has(id)) → true && "d" && {a,b}.has("d") → false. Falls through to the else branch: options?.preserveSet ? new Set<string>() : ... → empty set. Store: id=d, ids=∅.
5. React re-renders. useTimelineSelectionPreviewSync.ts:63-66 — selectedIds = orderSelectedIds(∅, "d") = [] (per :25-29, anchor not in ids returns the raw empty array). selectedKey = "". Effect re-runs.
6. previousSelectedKey = "a\0b" (from step 2). currentDomEditSelection = d (refs just updated), currentSelections = [d] (group.length===1 branch at :80-85), currentIds = [tl_id_of_d].
7. Branch at :95-107: selectedIds.length === 0 → true, previousSelectedKey.length > 0 && currentIds.length > 0 → true → applyDomSelection(null, { revealPanel: false }). Canvas cleared.

The mirror's docstring at domSelectionTimelineMirror.ts:53-57 acknowledges the "collapse otherwise" behavior is the fresh-click semantic ("what a fresh click means") but the empty-set path in useTimelineSelectionPreviewSync doesn't distinguish "timeline was intentionally cleared" from "single-select via preserveSet just emptied the set as a side effect."

Test coverage: useDomSelectionSelectionGuards.test.ts:242-278 verifies the store lands at id=d, ids=∅ (matching this trace, expected). No test in useTimelineSelectionPreviewSync.test.tsx transitions through the selectedElementId=X, selectedElementIds=∅ shape — the "clears preview selection when the timeline selection set is empty" case at :108-144 goes {clip-1} → ∅ with selectedElementId=null on the way, which is a different shape (null anchor, no orderSelectedIds non-membership fall-through).

Not tagging as a blocker because I haven't reproduced it in-app — Miga's manual test plan is a marquee-then-URL-restore flow, not a multi-select-then-single-click transition. But the trace is precise and the state chain matches the code you're shipping, so worth verifying before land. Two ways to close:

• In the mirror: for single-select with preserveSet:true, also call setTimelineSelectionSet(new Set([anchor])) when the anchor is NOT in the current set — collapse the set to just the anchor rather than emptying it. Would make the "fresh click" collapse mean {d} instead of ∅.
• In the store: options?.preserveSet ? new Set([id]) : ... in the else branch of setSelectedElementId — collapse to {id} on non-member preserveSet rather than empty. Small semantic change with a wider blast radius; the mirror change is safer.

Primary vs member resolution divergence in applyUrlSelection. useStudioUrlState.ts:154-155 — the URL restore resolves the primary via findElementForSelection(doc, primarySelection, activeCompPath) (which honors selection.sourceFile and hf-id fallback per domEditingElement.ts:249-267), but resolves each member via raw doc.getElementById(memberId) — no source-file filter, first document match wins, no hf-id fallback. Two sub-compositions flattened into the same document that share an authored id (a case domEditingElement.ts:263 explicitly acknowledges) restore the primary correctly bound to source A's element but pick source B's homonym for a member. Silent multi-selection contamination from a link. Also: if a URL-injected member id equals the primary id, it appears twice in the group array.

Group-drag snap-travel gate silently added. groupDragMove.ts:39 — if (!snapEngagedForTravel(proposed.dx, proposed.dy)) return proposed; early-returns before resolveSnapAdjustment and snapGuidesRef.current = .... The pre-PR inline group path in useDomEditOverlayGestures.ts had no such travel gate. The single-element path at :180 gains the same gate — matching in shape — but the group path change is a real behavior shift: pointer motions under 4px no longer engage snap for group; guides drawn on the last engaged frame can linger until the next engaged frame because the ref isn't cleared. Not a bug, but named because there's no test in groupDropKeepsSelection.test.ts pinning either the old or the new behavior.

Nits

Marquee-outside-canvas test covers the wrong half. marqueeOutsideCanvas.test.ts only exercises rectsOverlap with negative coordinates — rectsOverlap was never the bug; the actual fix is removal of the if (inComp) gate in DomEditOverlay.tsx:393-416. That fix has no test — refactoring the guard back accidentally would not fail anything in the current suite. A test that mounts DomEditOverlay, mousedowns outside the composition frame, and asserts a marquee starts would pin the real invariant.

rotationDegreesFromMatrix reports 180° for a mirrored ancestor. domEditOverlayGeometry.ts:169-174 decomposes atan2(b, a). An ancestor with scale(-1, 1) (or the 2D projection of rotationY: 180, which the drag test file at manualOffsetDrag.test.ts explicitly targets in adjacent scenarios) yields a=-1, b=0 → 180°. orientedOverlayRect feeds this angle into chrome (DomEditSelectionChrome.tsx:159, 234 — resolveRotatedResizeCursor(handle, 180)). Under the mirrored ancestor: resize cursors show flipped 180° (NW ↔ SE), rotate handle drawn at the bottom instead of the top. Pre-PR the ancestor walk didn't exist, so angle stayed at the element's own (typically identity, 0°) and cursors were correct.

Ancestor walk in readElementTransformSnapshot is O(depth) getComputedStyle per RAF. domEditOverlayGeometry.ts:151-156 walks every ancestor to <html> calling win.getComputedStyle per node — runs on every orientedOverlayRect invocation, i.e. every RAF while a selection exists. Not asset-flicker-vicinity (no memo/render/list-key impact), just per-frame cost proportional to DOM depth. Doc comment claims a data-composition-id stop condition but the loop doesn't enforce one; would be nice to actually short-circuit there.

applyUrlSelection isn't cancellable across rapid hashchanges. useStudioUrlState.ts:147-162 — the IIFE isn't guarded by a request-generation ref, so two hashchange events in flight can interleave and the later hash's applyMarqueeSelection can be overwritten by the earlier one that finished second. Return value is true synchronously ("doc ready") while the real work is still racing.

lastGroupPositions in createGroupDragMover leaks across gestures. groupDragMove.ts:28 — closure lives with the mover instance created once at useDomEditOverlayGestures.ts:100; lastGroupPositions = {} never resets when a new group drag starts. Debug-only (feeds findNonRigidMembers → logDrag("drift", ...)), but drift logs on frame 1 of gesture N reflect gesture N-1's final positions. Reset in the mover's start path.

Debug channels have no discoverable index. studioDebug.ts:1-3 documents opt-in via hf-<name>-debug but the only way to know select/drag/reload/resize are the valid <name>s is to grep for makeStudioDebugLogger(. A one-line // Live channels: reload, select, drag, resize (resize still hand-rolled — pending migration) at the top of studioDebug.ts would prevent silent typos (hf-selection-debug never fires and nothing complains).

resizeDebug.ts was NOT migrated to the factory. resizeDebug.ts:5-24 still hand-rolls the same hf-<name>-debug contract that makeStudioDebugLogger now owns. Cleanup-in-follow-up territory; naming so it doesn't get lost.

logDragMove increments moveN before checking enabled (dragDebug.ts:15-18) — sibling resizeDebug.ts:27-31 gates with if (!isEnabled()) return first. Cheap; consistent with the lazy-thunk invariant this PR-family cares about.

logDragSettle builds readDragPositions(elements) eagerly (dragDebug.ts:73-83) — schedules three setTimeouts on the iframe window regardless of hf-drag-debug state. Factory supports thunks; use them.

What lands cleanly

• Mirror + members-only-when->1 shape — the docstring at domSelectionTimelineMirror.ts:12-23 is exactly the kind of "why this ordering matters" note that saves a future contributor from a rediscovery cycle; the concern above is a case where it noticed the shape but the composing-hook doesn't handle the single-select-collapse output cleanly. Same shape as [[feedback_dispatch_first_ordering_race]].
• selectionIdsMatch in both directions — useTimelineSelectionPreviewSync.ts:31-49 — the comment "Compare as sets in BOTH directions: length equality misreads duplicates" is precisely right; two DOM children resolving to the same clip id would falsely match on a length check.
• Losing-primary re-resolves the group before wiping — useDomSelection.ts:411-421 — right shape for "a group is a set, not a scalar with an anchor," matches the [[project_hf_hf_id_contract_end_to_end]] primary-key-plus-fallback pattern.
• Ancestor-transform composition for selection geometry — domEditOverlayGeometry.ts:142-161 — the compose walk is the correct fix for "selection box sized under the wrong transform"; nit above about getComputedStyle cost, but the shape is right.
• Debug-factory lazy-thunk preserved — studioDebug.ts:9-27 — the #3137 R1 lazy-thunk fix survives the refactor into the shared factory.
• URL selGroup round-trips — encoding at studioUrlState.ts:149-151, decoding at :74-80, test at studioUrlState.test.ts:160. Members dropped rather than failing the others is the right resiliency default; concern above about primary-vs-member resolver divergence remains.

What I didn't verify

• Manual reproduction of the empty-set-clear-canvas trace in the running Studio (would need a live iframe + user gesture sequence; the analysis is code-level).
• Whether applyMarqueeSelection from a URL restore that yields members=[primary] produces different post-restore group state than applyDomSelection(primary, {revealPanel:false}) in practice (see useStudioUrlState.ts:158-160).
• CI has one CodeQL JS analyze + Windows-render/tests IN_PROGRESS at review time. Full green wasn't reached before I read.

Series note: #3138 sits on #3137's write-receipt base; the whole selection surface benefits from the write-token discipline downstream (refreshDomEditSelectionFromPreview re-resolving after receipts consumed). Preview-fix slice reads well as a batch: #3137 (write-receipts) → #3138 (selection) → #3139 (drag one-request) → #3140 (resize hold). LGTM from my side; the top concern above wants a look before land.

— Review by Rames D Jusso

[vanceingalls]

R1 review

Verdict: APPROVE (high confidence). Well-crafted PR — Miguel identifies six distinct multi-selection defects (shift-click misresolution, marquee drops non-first, group erases on timeline sync, dropped group deselects, marquee refuses to start off-canvas, URL loses group) and lands them as small, testable, individually-motivated commits. The core durability contract is sound: the timeline store is single-source-of-truth, and announceTimelineSelection publishes the members set BEFORE the anchor call so preserveSet: true finds the anchor in-set and preserves rather than empties. Two prerequisite geometry fixes (transform accumulation + measured drag mapping) are ordered first, which is right — the rest depends on the overlay reflecting where the element actually paints.

P1 blockers

None.

P2 non-blockers (follow-ups, not blocking)

P2-1. Anchor-null still empties the just-published members set — packages/studio/src/hooks/domSelectionTimelineMirror.ts:644-645.

The mirror publishes members, then calls setSelectedTimelineElementId(anchor, { preserveSet: true }). The store's implementation (playerStore.ts:526-533) treats preserveSet: true with id === null as "wipe the set" (options?.preserveSet ? new Set<string>() : ...). Scenario: user has A selected; shift-clicks span C where C has no timeline-row ancestor. members = [A_id], anchor = null. First call publishes {A_id}. Second call — id is null → falls through the guard to new Set<string>(). useTimelineSelectionPreviewSync.ts:1035 then sees selectedIds.length === 0 while currentIds.length > 0 and clears the canvas.

Miguel's mirror comment acknowledges this in-code, and behavior matches pre-PR for the same case, so it is not a regression. But it also is not fixed by the durability rework. Cheapest fix: if (anchor === null) return; after publishing the set, or a keepSet: true store option. Follow-up ticket, not a blocker.

P2-2. Ref mutation during render — packages/studio/src/hooks/useDomSelection.ts:474.

refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; written during render to break mutual-recursion between the single-refresh and group-refresh callbacks. Discouraged by React docs (concurrent-mode-hostile). Safe in practice because the assigned value is stable via useCallback and the ref is read only from event handlers, but strict-mode double-render or a discarded render could theoretically leave the ref pointing at a stale closure. Fix: wrap in useLayoutEffect(() => { ref.current = cb; }, [cb]). Follow-up.

P3 nits

• packages/studio/src/components/editor/domEditOverlayGeometry.ts:2270-2275 — walk is for (…; node; node = node.parentElement), which continues past the [data-composition-id] root all the way to documentElement. Comment says "The walk stops at the composition document's root" — code doesn't. In practice compositions don't transform <body>/<html>, so the difference is nil. Update comment or add a node === compositionRoot break.
• Same site — readElementTransformSnapshot is called every frame during drag via orientedOverlayRect, and now issues getComputedStyle(ancestor) for each parent. For a 5-deep DOM in a 3-member group that's ~15 style reads/frame. Likely fine but worth a spot-check on the heavier regression-shards scenes.
• packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts — asserts rectsOverlap, but the behavior change is in handleOverlayPointerDown at DomEditOverlay.tsx:1655-1678 (dropped the inComp guard). Consider an integration test that drives a pointer down at negative overlay coordinates and asserts the marquee ref gets seeded.
• packages/studio/src/utils/dragDebug.ts:76-82 — logDragSettle unconditionally schedules three setTimeouts per drop even when the debug flag is off; log calls inside are no-ops but the timeouts still schedule and their closures pin elements. Guard with isEnabled or move the schedule inside the logger callback.
• packages/studio/src/utils/studioUrlState.ts:187-189 — selGroup is JSON-serialized. URLs with 20+ selector-based members can approach the 2000-char browser cap. Not a blocker; a compression scheme is the follow-up if it hits.

Per-lens findings

1. Durability semantics — Persist across shift-add, marquee, timeline sync (via preserveSet+members-first ordering), URL hash (via selGroup JSON), preview-doc replacement (via refreshDomEditGroupSelectionsFromPreview), group drop (via suppressNextBoxClickRef). All four channels traced. Only edge case is P2-1.
2. Selection-set data structure — Set<string> keyed by timeline element ID (via findMatchingTimelineElementId + findTimelineIdByAncestor fallback). Both id-based and selector-based DOM selections resolve to the same timeline ID when they share a row ancestor. No issue.
3. Element-identity keying — Post-e8a3e3a, URL group is Array<{sourceFile, id, selector, selectorIndex}> — id-less selector-based members round-trip. Prior commit's groupIds: string[] gap was the same shape as the flat-inspector cluster; Miguel closed it in the final commit. No remaining el.id ?? el.selector shorthand. No issue.
4. Concurrent-mutation safety — useDomEditPreviewSync.ts:1195-1198 catches "primary lost, group survived" and re-resolves. refreshDomEditGroupSelectionsFromPreview drops missing members and keeps survivors. Symmetric with URL restore. No issue.
5. Range/marquee interaction — Marquee gate removed; hit-test was always in overlay space and never clipped. useMarqueeGestures still owns marquee lifecycle. No regressions.
6. useEffect state-syncing — No new useEffect(setState, [prop]) patterns.
7. Fast Refresh / HMR — New non-component modules export functions only.
8. Undo/redo — Not touched; selection is not part of history.
9. Test coverage — Strong; asserts on persisted store side-effects. Gap: no integration test for off-canvas marquee-start (P3).
10. CI — 27 in-progress at review time; prior head at 92c8121 was all-green on comparable checks. No failures.

— Via

[vanceingalls]

R2 re-verify @ 9e3ac930e

Verdict: APPROVE. Both P2 findings fixed with well-thought-out approaches. Most P3 nits also fixed. Clean pass.

Per-finding delta:

• P2-1 (anchor-null wipes just-published members): FIXED. packages/studio/src/hooks/domSelectionTimelineMirror.ts now computes timelineAnchor = anchor ?? members[0] ?? null, publishes the union set, then if (!timelineAnchor) return; before the setSelectedTimelineElementId(anchor, { preserveSet: true }) call. Canvas-only primary with surviving timeline members → falls back to members[0] and preserveSet works; canvas-only primary with no timeline members → early return preserves the existing set. Richer than my R1 suggestion (just short-circuit) — keeps the timeline visualization coherent when some members still have rows. Confirmed against useDomSelectionSelectionGuards.test.ts regression: fresh single-click on a non-member now collapses to that one id rather than emptying the set. Note: commit 7c837cea6a alone didn't fully close this; the complete fix arrived in 9e3ac930e6.
• P2-2 (ref mutation during render): FIXED. packages/studio/src/hooks/useDomSelection.ts:487-489 moves the assignment into useEffect(() => { refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; }, [refreshDomEditGroupSelectionsFromPreview]). Uses useEffect rather than my suggested useLayoutEffect; safe here because the only consumer (line 430) reads inside an async user-event-driven callback that always runs after commit.

P3 nits status:

• Composition-root walk stop drift: FIXED — if (node.hasAttribute("data-composition-id")) break; now matches the doc-comment.
• getComputedStyle(ancestor) per-frame cost: unchanged; cost now bounded by the added root-stop, but not cached across frames. Informational only.
• marqueeOutsideCanvas.test.ts asserting wrong thing: FIXED via deletion; behavior now covered by broader tests in the selection guards suite.
• dragDebug.ts:76-82 unconditional setTimeout: FIXED — logDragSettle now wraps the setTimeouts inside logDrag(stage, () => { ... }) which short-circuits on !enabled before invoking; same treatment for logDragMove.
• studioUrlState.ts selGroup URL length: unchanged — actually slightly worse since group members now serialize {sourceFile, id, selector, selectorIndex} rather than bare ids (~3-10× per member). Legacy CSV fallback preserves back-compat. Small/medium groups safe; 25+ members with long selectors can approach older-browser caps. Informational.

Two info-tier observations from the incremental diff (non-blocking):

1. timelineAnchor = anchor ?? members[0] picks whichever ordering group.map(timelineIdFor) produces when canvas primary lacks a timeline row; if group order isn't stable across re-runs, displayed anchor could differ across mounts. Not a correctness bug — just noting the state-derivation dependency.
2. useEffect (vs useLayoutEffect) leaves an async gap between render and effect where refreshDomEditGroupSelectionsFromPreviewRef.current is still the initial no-op. Not exploitable given current call sites (all async, user-driven); worth documenting alongside the effect if a future caller reads it synchronously.

CI: all completed required checks SUCCESS; regression-shards and a few Windows/JS jobs still IN_PROGRESS. The four apparent fail rows in gh pr checks are matrix-template names from prior cancelled runs superseded by newer pushes — no real failures at head.

Stamp holds pending in-progress checks completing green.

— Via

[james-russo-rames-d-jusso]

Delta-reviewed 92c812131..9e3ac930e against prior R1 findings.

MAIN concern from R1 is fixed cleanly, at the right seam. The mirror in packages/studio/src/hooks/domSelectionTimelineMirror.ts:49-72 now sets the timeline set BEFORE anchoring — computes publishedMembers = new Set(members), force-adds the anchor at :52 (if (anchor) publishedMembers.add(anchor)), publishes the set with setTimelineSelectionSet(publishedMembers) at :70, then finally calls setSelectedTimelineElementId(timelineAnchor, { preserveSet: true }) at :72. That inverts the trap: the store's preserveSet gate at playerStore.ts:526 (Boolean(options?.preserveSet && id && s.selectedElementIds.has(id))) now returns TRUE because the id was just added to the set, so the else-branch that empties the set is unreachable for the fresh-click case. The doc block at :13-24 names the fix in its own words ("Announcing only the primary therefore drops every other member. Worse, anchoring with preserveSet on an id the set does not yet contain empties the set outright, and an empty set syncs back as 'nothing is selected'"). Trace: marquee [a,b] → ids={a,b}; single canvas-click on unrelated d → mirror sets {tl_d}, then anchor to tl_d with preserveSet, store has(tl_d) true, ids stays {tl_d}, sync sees selectedIds=[tl_d] non-empty, no wipe. ✅

Also confirmed: the gate at :69 (if (group.length > 1 || !getTimelineSelectionSet().has(timelineAnchor))) means a late async primary that already belongs to the live set (post-fetch resolution of a preserved anchor) skips the set-publish and just re-anchors — that's what the comment at :66-68 is protecting.

One subtle new risk and a few smaller notes below.

Concerns

Canvas-only primary + timeline-row group member → anchor fallback silently swaps the primary. domSelectionTimelineMirror.ts:50, 53 — when the primary is a canvas-only element with no timeline row, anchor = timelineIdFor(primary) = null. The fallback timelineAnchor = anchor ?? members[0] ?? null at :53 picks the first timeline-row member as the anchor when the primary has none. That publishes the member's id as the timeline anchor, and downstream useTimelineSelectionPreviewSync.ts:48-50 compares currentAnchor (resolved from the canvas-only primary → null) against wantedAnchor = members[0], detects the mismatch, and re-syncs — quietly replacing the user's canvas-only primary with the timeline-row member as the new primary. In practice: user selects a canvas-only element P and shift-selects a timeline-row element M as a group; the mirror silently promotes M to primary. The comment at :66-68 and the early-return at :65 (if (!timelineAnchor) return;) cover the "no timeline row at all" case cleanly, but the "primary has none, but at least one member does" case slides into the fallback path without saying so. Neither groupDropKeepsSelection.test.ts (which asserts suppressNextBoxClickRef.current === true, not primary-preservation) nor the new "canvas-only anchor fallback" test in useDomSelection.test.ts:148-173 (asserts store-call args, not downstream sync) covers this transition. Cheap fix: either preserve the null anchor for the mixed case (falling through the early-return) and let the store carry a null-primary + non-empty-set, or announce the primary-swap intentionally in a comment so the next contributor knows the fallback is by design.

What Miga fixed cleanly

URL primary/member resolver divergence. useStudioUrlState.ts:104-120 — new findUrlSelectionElement delegates to findElementForSelection(doc, {sourceFile: target.sourceFile ?? fallbackSourceFile, ...}, activeCompPath). resolveUrlSelections at :144-149 threads fallbackSourceFile = selection.sourceFile ?? "" (the primary's sourceFile) into every member lookup. Raw doc.getElementById is gone. Homonym collision across source files is closed.

Silently-added snap-travel gate now called out. snapEngine.ts:5-22 defines SNAP_ENGAGE_TRAVEL_PX = 4 and snapEngagedForTravel(dx, dy) with a doc block explaining the "pick a selection up and it teleports" bug; applied in groupDragMove.ts:40-43 and via disabledForTravel on resolveSnapAdjustment at snapEngine.ts:381-385. Direct test at snapEngageTravel.test.ts (66 lines). Named in commit 1d36f3ea0: "fix(studio): stop snapping from moving a selection you have not dragged yet". Fix and provenance both there.

Nits — verdict per prior R1 item

• marqueeOutsideCanvas.test.ts removed — FIXED (consolidated). Case moved to DomEditOverlay.test.ts:331-366 ("starts a marquee from outside the composition frame") which exercises the overlay component itself, not just the pure rectsOverlap helper — strictly better coverage.
• rotationDegreesFromMatrix on mirrored ancestor — FIXED. domEditOverlayGeometry.ts:171-185 now computes both fromX = atan2(b,a) and fromY = atan2(-c,d), and when determinant < 0 && Math.abs(fromY) < Math.abs(fromX) returns fromY. Comment explains the equivalence-under-reflection.
• Ancestor walk O(depth) getComputedStyle — NOT-FIXED. isElementVisibleThroughAncestors at domEditingDom.ts:33-50 still walks parentElement calling win.getComputedStyle at every level. Nothing in R2 touches it. Fine to defer — it was a nit and hasn't shown up as a hot path.
• applyUrlSelection cancellable across hashchanges — FIXED. useStudioUrlState.ts:186 selectionApplySeqRef = useRef(0), :208 const applySeq = ++selectionApplySeqRef.current, then isCurrent: () => applySeq === selectionApplySeqRef.current (:241) threaded into resolveUrlSelections; bails at :140 and :151 on stale seq. Monotonic counter, no AbortController — same semantics.
• lastGroupPositions leaks across gestures — FIXED. groupDragMove.ts:28-29 holds the map + gesture in the mover's closure; :74-77 resets on gesture identity change (if (groupG !== lastGesture) { lastGesture = groupG; lastGroupPositions = {}; }).
• resizeDebug not migrated to factory — FIXED. resizeDebug.ts:4-6 now import { makeStudioDebugLogger }, export const logResize = makeStudioDebugLogger("resize"). Matches studioDebug.ts:13-30.
• logDragMove / logDragSettle build-before-check — FIXED. dragDebug.ts:15-19 logDragMove passes an arrow-thunk () => { moveN += 1; return moveN % 8 === 1 ? {...} : null; }; studioDebug.ts:23-25 gates on if (!enabled) return; before invoking. logDragSettle at :79-97 also arrow-thunk, only schedules the +120/+400/+900ms telemetry setTimeouts when enabled. Same pattern for logResizeMove / logResizeSettle.

What lands cleanly

• Members-before-anchor mirror order (domSelectionTimelineMirror.ts:49-72) with a doc block that names the trap it closes.
• Only one production call site of preserveSet: true in the codebase — the mirror — and it is now safe by construction. Test call-sites in useDomSelection.test.ts are contract locks, not new callers.
• snapEngageTravel.test.ts:55-63 verifies snap still applies when disabledForTravel is unset, so the gate doesn't accidentally disable snap in the intended paths.

The subtle-swap concern in the canvas-only-primary + timeline-row-member case is the only thing worth naming here; everything else lands cleanly.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

Re-verified bc1c77bb4 against prior R2 (9e3ac930e).

Every file in this PR's own scope is byte-identical to the prior R2 head — the delta at this SHA is stack drift from the fileWatcher fix that landed in #3137, nothing in #3138's own scope moved. Prior R2 verdict stands unchanged: main R1 concern (single-canvas-click after multi-select wiping canvas) fixed cleanly via publish-set-before-anchor in domSelectionTimelineMirror.ts:49-72; one subtle new risk in the canvas-only-primary + timeline-row-member mixed case (fallback timelineAnchor = members[0] silently promotes member to primary through the sync hook) named at review 4892125122 — non-blocking follow-up.

No new findings.

— Review by Rames D Jusso

[vanceingalls]

R3 re-stamp @ bc1c77b — Byte-clean rebase on #3137 R3 (f1de808). Own-delta against parent (f1de808e7...bc1c77bb4) = 33 files with blob SHAs and +/- counts identical to R2 own-delta (4d2a48ae6...9e3ac930e); no code change to #3138 introduced by the rebase, only the fileWatcher burst-fix propagated up from #3137. Prior R2 approval stands. — Via

[miguel-heygen]

Landed in #3146 — same commits, on an unstacked branch. GitHub refuses base retargeting and admin merge on a stacked PR, and the async merge endpoint it does allow does not apply the ruleset bypass, so this could not be merged in place. All content is on main.