feat(studio): persist element positions in HTML, fix resize overlay drift and GSAP double-translation

[miguel-heygen]

Summary

• Positions persisted directly to HTML inline styles — no more .hyperframes/studio-manual-edits.json sidecar file
• "Manual positioning" toggle removed — every element with drag/resize/rotate capability works immediately, no opt-in required
• No re-apply step on page load — CSS custom properties baked into the HTML source survive page refreshes automatically
• Seek reapply updated — reapplyPositionEditsAfterSeek queries data-hf-studio-* markers to re-compose translate/rotate/width/height after GSAP seek
• Fix: position drift after drag + reload — GSAP baked the resolved CSS translate value into element.style.transform on every seek, causing double-translation when the studio's reapply hook also wrote the offset via translate individual property. After a drag to 259px, reload would show 518px instead. stripGsapTranslateFromTransform now zeros the translate component from GSAP's matrix (preserving scale/rotate) after every offset apply.
• Fix: resize overlay misalignment with scaled elements — elements with a GSAP scale: 0.85 (center transform-origin) drifted visually when CSS width/height changed because the scale shifts the visual left edge. Drift compensation measures getBoundingClientRect before/after the resize and adjusts left/top to cancel the shift.
• Fix: rendered video ignores studio drag/resize/rotation positions — the seek-reapply script used setInterval to wrap window.__hf.seek, but Puppeteer's page.evaluate() never yields the event loop for macrotasks so the interval never fired. Now uses Object.defineProperty to trap writes to the seek property, wrapping it synchronously the instant the bridge assigns it.
• Fix: video proxy missing CSS individual transform properties — MEDIA_VISUAL_STYLE_PROPERTIES included transform but not translate, rotate, or scale (CSS Transforms Level 2). The proxy <img> for <video> elements was positioned at offsetLeft/offsetTop without the studio translate offset. Also fixed getViewportMatrix for HDR elements.
• Fix: preview hot reload on external file changes — the SSE/HMR file-change handler in useManifestPersistence handled motion manifest paths but did nothing for other files (index.html, assets). Now calls reloadPreview() for non-motion external changes, with echo suppression via the shared domEditSaveTimestampRef.
• Fix: elements with pointer-events: none unselectable in Studio — elementsFromPoint() skips these elements. Now temporarily injects * { pointer-events: auto !important } during hit-testing. Added pointer_events_none lint rule (info severity).
• Fix: post-resize click selects wrong element — after pointer-up on resize handle, the click event bubbled to handleBoxClick and re-selected whatever element was under the pointer. Now suppresses the click in both resize exit paths.

Fixes #854

How it works

When you drag an element, the live DOM already gets --hf-studio-offset-x/y and translate: var(...) as inline styles (unchanged). The new path then writes those same values into the HTML source file using persistDomEditOperations with skipRefresh: true — one save per drag gesture, coalesced by key, no iframe reload.

Reset edits clears the custom properties and restores original inline values, then reloads so CSS classes re-assert.

GSAP double-translation fix: GSAP 3.x reads the resolved translate CSS individual property value at tween initialization and caches it into its internal transform matrix. On every seek(), it writes this cached translate back into element.style.transform. When the studio's reapply then also sets element.style.translate = var(--hf-studio-offset-x, ...), both properties contribute — doubling the visual offset. The fix strips only the translate component (m41/m42) from the GSAP-managed matrix using DOMMatrix, leaving scale, rotate, and skew untouched.

Render position fix: The seek-reapply script (studioPositionSeekReapplyRuntime) wraps window.__hf.seek so that reapplyAll() runs after every GSAP seek to restore CSS translate individual properties. The wrapping used setInterval polling, but during Puppeteer rendering page.evaluate() calls don't yield the event loop for macrotasks — the interval never fired. Now uses Object.defineProperty to install a setter trap on window.__hf.seek, wrapping the function synchronously the instant the bridge script assigns it.

Changes

• sourcePatcher: PatchOperation.value: string | null — null removes the property/attribute from the tag
• manualEditsDom: build*Patches / buildClear*Patches helpers serialize live element state to PatchOperation[]; reapplyPositionEditsAfterSeek replaces manifest-based reapply; stripGsapTranslateFromTransform fixes double-translation; resize drift compensation in applyStudioBoxSizeDimensions
• manualEdits.ts: remove applyStudioManualEditManifest, all manifest target resolution; keep seek/play wrap infrastructure
• useManifestPersistence: remove all JSON I/O, manifest state, toggle state; seek hook uses reapplyPositionEditsAfterSeek; SSE handler now calls reloadPreview() for non-motion external file changes
• useDomEditCommits: new commitPositionPatchToHtml replaces commitStudioManualEditManifestOptimistically
• DomEditOverlay, PropertyPanel, context/session/App wiring: remove manualEditsEnabled entirely
• manualEditsParsing / manualEditsTypes: remove manifest types and upsert functions
• manualEditsRenderScript: installSeekTrap via Object.defineProperty replaces setInterval polling for wrapping seek functions
• parityContract: add translate, rotate, scale to MEDIA_VISUAL_STYLE_PROPERTIES
• videoFrameInjector: getViewportMatrix composes CSS individual transform properties (translate, rotate, scale) into the viewport matrix
• studioPreviewHelpers: getPreviewTargetFromPointer forces pointer-events: auto during hit-testing
• useDomEditOverlayGestures: suppress post-resize click in both resize exit paths
• lint/rules/core: add pointer_events_none info rule

Test plan

• Drag an absolute-positioned element → source HTML gets --hf-studio-offset-x/y + translate inline; no .hyperframes/studio-manual-edits.json created
• Drag a relative-positioned element → same HTML write
• Reload page → element stays at dragged position (no drift, no doubling)
• Drag element on a composition with GSAP scale tween → position persists correctly after reload
• Seek animation → positions preserved (seek hook re-composes translate from custom props)
• Resize element with GSAP scale transform → overlay stays aligned, no left-edge drift
• Reset edits → inline styles stripped, page reloads, elements return to original
• Undo (Cmd+Z) → positions restored
• Render via CLI → element positions match studio preview (drag, resize, rotation all preserved)
• Render with transparent WebM video → video proxy positioned correctly with studio translate offset
• Edit file in VS Code → studio preview hot-reloads automatically
• Resize element → same element stays selected (no wrong-element selection)
• Click on element with pointer-events: none in composition → element is selectable in Studio
• hyperframes lint --verbose → reports pointer_events_none info findings
• bun test in core, engine → all tests pass

[jrusso1020]

LGTM — opt-out toggle is well-scoped, state machine is clean.

Audited (deep read)

• useManifestPersistence.ts — new manualEditsEnabled state + setManualEditsEnabled setter. Initial value false; reset to false on project switch; rehydrates from parsed.enabled ?? false on manifest read. Optimistic update + queued save with rollback on error (restores previous manifest, bumps revision, restores local state, shows toast). Coalesce key "manual-edits-enabled" is appropriate. ✓
• manualEditsParsing.ts / manualEditsTypes.ts — strict record.enabled === true parse (1/"true" rejected); optional in serialized form. ✓
• PropertyPanel.tsx — needsToggleForElement = !canMove && !manualEditsEnabled gates manualOffsetEditingDisabled, manualSizeEditingDisabled, and the rotation field's disabled prop. Toggle renders at bottom of panel in both empty + selected states, conditionally on onSetManualEditsEnabled being passed. Switch component has correct role="switch" + aria-checked semantics. ✓
• DomEditOverlay.tsx — drag gate if (!candidate.capabilities.canMove && !manualEditsEnabled) return; short-circuits gesture-init for locked elements; cursor styling and onPointerDown both honor canMove || manualEditsEnabled. ✓
• usePreviewInteraction.ts — blocked-move toast now distinguishes "toggle would unlock this" (canApplyManualOffset && !canMove) from genuinely-disabled cases. Message text points users at the Design panel. ✓
• DomEditContext + useDomEditSession + App.tsx + StudioRightPanel.tsx + StudioPreviewArea.tsx — pass-through plumbing for manualEditsEnabled / setManualEditsEnabled. Memo deps updated correctly in DomEditProvider. ✓

Non-blocking — path-enumeration on the new state-machine contract

The toggle defines a new invariant: "a non-canMove element can only have its position/size/rotation mutated when manualEditsEnabled=true." The diff gates three sites — drag gesture, property-panel form fields, and the blocked-move toast. Worth confirming the following paths (not visible in this diff) also honor the gate, or are intentionally exempt:

• Arrow-key nudges (if there's a keyboard handler that translates selection)
• Resize handles (if there's a separate handle layer beyond the drag-anywhere gesture)
• Group/multi-select drag (groupSelections is passed through but I don't see a per-selection gate in the gesture path)
• Programmatic offset application from agent/AI edits (e.g., a Magic Edit chat result writing to a non-canMove element)

If any of these can mutate a non-canMove element without the toggle, the user-visible promise ("turn this on to move it") is partially defeated. Probably worth a quick grep on applyManualOffset / setStudioPathOffset callers to confirm coverage.

Non-blocking — scope smell

The branch name (feat/studio-preview-pasteboard-bg) and the Player.tsx + NLEPreview.tsx changes (transparent player container + shadow-root style injection adding .hfp-container{overflow:visible} + canvas drop-shadow, viewport bg-neutral-700) are pasteboard-aesthetics work, not part of the manual-positioning toggle. Reviewable in one pass, but in the future it'd be cleaner to keep these as separate PRs — both for review ergonomics and so a revert of one doesn't drag the other.

Non-blocking — CI

Test job failed on both runs (latest at job 75935191801) and Tests on windows-latest also failed. Build/Lint/Typecheck/Smoke all green. Worth a quick look at the test logs before merge — could be a snapshot/visual regression triggered by the new pasteboard bg, or a flake.

Nit

• PropertyPanel.tsx line ~182: manualEditsEnabled = true as the default-destructured value is inconsistent with the hook's false initial state. Cosmetic only since App.tsx always passes the real value — but the default would render the toggle as "on" if the prop were ever undefined. Consider manualEditsEnabled = false to match the source of truth.

— Review by Rames Jusso (pr-review)

[vanceingalls]

Net-new opt-out toggle for manual positioning; absolute-positioned elements stay always-draggable, JSON sidecar is created lazily. Behaviour matches what the PR description promises in the simple case, but the gate hasn't been propagated to every site that satisfies the contract, and one required CI check is red.

Strengths

• packages/studio/src/components/editor/manualEditsParsing.ts:129 — enabled === true (default-false) is the correct shape for opt-in semantics on legacy manifests that lack the field.
• packages/studio/src/hooks/useManifestPersistence.ts:485-517 — setManualEditsEnabled does optimistic UI + rollback-on-failure with a coalesce key and history label. Good shape.
• packages/studio/src/hooks/usePreviewInteraction.ts:127-130 — targeted toast wording ("Enable 'Manual positioning' in the Design panel to move this element.") is discoverable and points at the exact remedy.

Blockers

1. Required CI Test check is failing — packages/studio/src/components/editor/DomEditOverlay.test.ts:177 "DomEditOverlay > renders selected bounds right after clicking a movable selection" asserts currentSelection === selection and gets null. Root cause: the new gate at DomEditOverlay.tsx:215-216 (if (!candidate.capabilities.canMove && !manualEditsEnabled) return;) short-circuits because the test fixture omits canMove (undefined → falsy) and the harness doesn't pass manualEditsEnabled (defaults to false per the new prop default at DomEditOverlay.tsx:71). Fix: add canMove: true to the test capabilities fixture (the fixture already says position: "absolute", so this is the value resolveDomEditCapabilities would compute in the real path). The PR shipped a regression that the suite is catching — please confirm green before merge. The same failure repeats on Tests on windows-latest.

2. Multi-select drag bypasses the toggle entirely — DomEditOverlay.tsx:173-176 derives groupCanMove from every((item) => item.selection.capabilities.canApplyManualOffset) with no canMove || manualEditsEnabled factor, and gestures.startGroupDrag(e) at line 314 fires unconditionally once allowCanvasMovement is true. Failure mode: shift-click two non-absolute elements with the toggle off, drag the group — both move and write to the JSON sidecar that the user explicitly opted out of. This is the exact scenario the PR description claims is locked.

3. Single-element resize and rotate handles still render and fire for non-absolute elements when the toggle is off — DomEditOverlay.tsx:322 (rotate handle) gates only on selection.capabilities.canApplyManualRotation; DomEditOverlay.tsx:388-396 (resize handle) gates only on selection.capabilities.canApplyManualSize. Neither factors canMove || manualEditsEnabled. The PR description states "Drag handles, resize, and rotation handles are hidden when disabled" — drag is, the other two aren't. Same data corruption surface as #2: user thinks they're opted out, dragging the resize/rotate handles writes JSON edits silently. (Note: PropertyPanel.tsx's R-field IS toggle-gated via needsToggleForElement — the canvas handle is the asymmetric one.)

4. Defence-in-depth gap at the gesture boundary — DomEditOverlay.tsx:719-721 (startGesture for drag/resize/rotate) checks canApplyManual{Offset,Size,Rotation} but not the toggle. If any caller path slips through #2 or #3, startGesture happily proceeds. Add the canMove || manualEditsEnabled precondition here so this is the single chokepoint instead of N call-sites each having to remember.

Important

• Test coverage gap on the new behaviour — the toggle's effect is structural (drag/resize/rotate gated by canMove || manualEditsEnabled) but no test in this PR pins it. After fixing #1, please add: (a) toggle-off + non-absolute element + drag attempt = blocked-move toast, (b) toggle-off + absolute element + drag = succeeds, (c) toggle-off + non-absolute + group drag = blocked (addresses #2), (d) toggle-on persists to .hyperframes/studio-manual-edits.json with enabled: true. Without these, future refactors to the gate logic will silently regress.

• Toggle accessibility — PropertyPanel.tsx:222-244 uses role="switch" + aria-checked (good) but no aria-label / no associated label element. The visible text "Manual positioning" lives in a sibling <span>. Screen readers will read the switch state without context. Wrap or aria-labelledby to the span.

• Cross-project preference vs per-project — toggle state writes to per-project .hyperframes/studio-manual-edits.json (good, matches "people who want to share source can opt in their project"). But the default-off for newly-loaded existing projects without the field means anyone who relied on the prior implicit-on behaviour on an existing project that never wrote enabled: true will silently lose drag-to-move on next load. The PR body says "Existing projects that already have enabled: true in the manifest respect that value on load" — but pre-PR there was no enabled field at all, so no existing project has enabled: true. Worth a migration note or a one-time toast on first-load-after-update.

Nits

• useManifestPersistence.ts:443 — useState(false) for the in-memory default, then setManualEditsEnabledState(parsed.enabled ?? false) on load. The two falses drift if you ever want to flip the default — extract a MANUAL_EDITS_DEFAULT_ENABLED = false constant.
• Commits 4/5 (enable-by-default → revert to opt-out) flip the same constants three times. Squash-merge will hide it; leaving as-is is fine, just calling out for reviewer flow.

---

Verdict: REQUEST CHANGES
Reasoning: Required CI Test is failing on a test that exercises this PR's own new gate (blocker #1), and the toggle is bypassable via three other interaction sites (multi-select drag, canvas resize, canvas rotate) that violate the PR's stated invariant.

— Vai

[jrusso1020]

Recalibrating after Vai's review: agreeing with their request-changes call.

My non-blocking path-enum framing was too soft. The PR body explicitly promises "Everything else (relative, static, flex children, …) is locked by default" — that's a global invariant, and per the path-enumeration discipline for new state-machine contracts, every path that can mutate position/size/rotation on a non-canMove element must honor the toggle. Vai enumerated three bypasses (multi-select drag, resize handle, rotate handle) that defeat the user-visible promise. Those should be blockers, not nits.

Also revising my CI note: Vai pinned the Test failure to DomEditOverlay.test.ts:177 short-circuiting on the new gate (test fixture omits canMove). That's the PR's own change breaking an existing test — substantive failure, not a flake.

Treating my prior approval as superseded by Vai's request-changes. Happy to re-stamp once the bypass paths are gated and the test fixture is updated.

— Recalibration by Rames Jusso (pr-review)

[jrusso1020]

Re-approving on 414320a after the architectural pivot.

The PR has changed shape from yesterday's "manual-positioning toggle + JSON manifest" design to "every element with drag/resize/rotate capability moves immediately; positions persist directly to HTML inline styles." This matches the design conclusion the Slack thread converged on yesterday ("code should be source of truth, JSON is just debounced cache") and moots every blocker from the prior round — there's no toggle anymore, so the multi-select-drag / resize-handle / rotate-handle bypass concerns (Vai's #2-4) and the DomEditOverlay.test.ts:177 short-circuit (Vai's #1) no longer apply. CI is fully green now, including the test that was failing yesterday.

Audited on the new commit

• State-machine removal (useManifestPersistence.ts -136 net, manualEditsParsing.ts -238, manualEditsTypes.ts -39, manualEdits.ts -162) — the manifest persistence layer and the toggle state are gone. commitPositionPatchToHtml replaces commitStudioManualEditManifestOptimistically. Code shrinks substantially in the studio components; new logic centralizes in manualEditsDom.ts (361 LoC, mostly the patch builders + stripGsapTranslateFromTransform). ✓
• PatchOperation.value: string | null (sourcePatcher.ts:73-74) — null signals removal of the property/attribute. All four patch sites (patchInlineStyle, patchInlineStyleByTarget, patchAttribute, patchAttributeByTarget) handle null correctly:
  • For inline-style: when target tag has no existing style attribute, null is no-op; when it has one, the property is deleted from the parsed prop map and the style string is rebuilt. ✓
  • For attribute: removes the attribute (with leading whitespace, via \s+${escapeRegex(...)}=...) if present; no-op if absent. ✓
  • Also fixes a pre-existing minor: escapeRegex(fullAttr) is now applied in patchAttribute and patchAttributeByTarget, which it wasn't before — hardens against odd attribute names containing regex metachars. ✓
• stripGsapTranslateFromTransform (manualEditsDom.ts:215-238) — GSAP's matrix baking is the real root cause of the "drag to 259px then reload → 518px" symptom the body calls out. The fix:
  • Reads element.style.transform (the inline string, not computed) — GSAP writes resolved matrix here on each seek
  • Parses with DOMMatrix(transform) defensively (try/catch, DOMMatrix may be undefined off-window)
  • Skips if m41/m42 are already 0 (no-op guard)
  • Zeroes m41/m42, leaves m11/m12/m21/m22 (scale/rotate/skew) alone, leaves m43/m44 (3D) untouched
  • If result is a 2D identity matrix, removes the transform property entirely (clean reset); else writes back the matrix
  • Fires after every applyStudioPathOffset / applyStudioPathOffsetDraft so it's in lockstep with the studio's translate write ✓
• installSeekTrap via Object.defineProperty (manualEditsRenderScript.ts:99-218) — replaces the setInterval polling that never fired during Puppeteer's page.evaluate() because the event loop doesn't yield to macrotasks during synchronous bridge initialization. New approach:
  • Object.defineProperty setter trap on window.__hf.seek / window.__player.renderSeek — captures the assignment the instant the bridge writes to the property
  • On set, wraps the function so post-seek reapplyAll() re-composes translate/rotate from --hf-studio-offset-x/y and --hf-studio-rotation CSS custom properties
  • markWrapped uses non-enumerable, non-configurable defineProperty so wrapping is idempotent (avoids double-wrap)
  • Early-return if no studio-marked elements exist (skips the wrap install entirely on untouched HTML) ✓
• MEDIA_VISUAL_STYLE_PROPERTIES (parityContract.ts:27-29) — adds the three CSS Transforms Level 2 individual properties (translate, rotate, scale). The video proxy now copies these too, so a studio-translated <video> doesn't drop its offset when the engine creates the proxy. ✓
• composeIndividualTransforms (videoFrameInjector.ts:556-585) — composes the three individual properties into a DOMMatrix, then mat.translate(ox, oy).multiply(individualTransform).multiply(t).translate(-ox, -oy) — order is "origin shift → individual transforms → transform property → origin shift back." That matches the CSS spec order (individual transforms apply before transform). ✓
• getPreviewTargetFromPointer pointer-events:none hit-test (studioPreviewHelpers.ts:104-122) — temporarily injects * { pointer-events: auto !important } via <style> element, runs the hit-test, removes the style in finally. Synchronous insert/remove so other event handlers can't observe the override window. ✓

Body claims vs diff

Spot-checked:

• "GSAP baked the resolved CSS translate value into element.style.transform on every seek, causing double-translation" — verified, the fix is exactly the matrix m41/m42 zero ✓
• "setInterval polling never fired during Puppeteer's page.evaluate()" — Object.defineProperty setter trap is the right shape ✓
• "MEDIA_VISUAL_STYLE_PROPERTIES included transform but not translate, rotate, scale" — verified ✓
• "hyperframes lint --verbose reports pointer_events_none info findings" — new lint rule at lint/rules/core.ts +52 ✓
• Test plan checks all marked done. CI matches.

Non-blocking notes

1. composeIndividualTransforms parsing assumes computed-style px/deg output — parseFloat(parts[0] ?? "0") on translate works because getComputedStyle(el).getPropertyValue("translate") normalizes to absolute pixel values. Same for rotate (browsers normalize to deg). Hand-authored translate: 1em 2em in source HTML would be wrong if it reached this path uncomputed, but it can't — getComputedStyle always resolves. Worth a one-line comment noting "values are guaranteed to be in px/deg by getComputedStyle" so a future reader doesn't accidentally feed it raw source-side input.

2. pointer-events: auto !important injection global override — during the synchronous hit-test window the entire document briefly has pointer-events: auto, overriding any composition that intentionally uses pointer-events: none to layer click-through behavior. Synchronous removal means no other event handler observes it, so no functional leak. But: if forcePointerEventsAuto succeeds and resolveVisualDomEditSelectionTarget / getDomLayerPatchTarget throws or returns early before the finally removes it... the try/finally guarantees removal, so this is fine. Just noting it's a heavy hammer; alternatives (elementsFromPoint already returns elements regardless of pointer-events, so the new fallback path may be the better-targeted fix without needing the global override). Cosmetic.

3. Idempotency of __hf_studio_pointer_events_override__ style ID — if a prior hit-test leaked (e.g., due to a crashed render), the next call appends a second <style> with the same ID. style.remove() in finally would only remove the most recent. Practical leak is bounded since the try/finally is robust, but forcePointerEventsAuto could doc.getElementById(POINTER_EVENTS_OVERRIDE_ID)?.remove() first to defend. Cosmetic.

4. Scope — PR grew from yesterday's 189/30 to 1066/1305 across 29 files. The breadth is justified given the design pivot, but worth flagging that #854 (the render-position fix) and the GSAP double-translation fix and the pointer-events fix and the post-resize-click fix are all riding along — any one of them could have been its own PR. Reviewable as-is, but the historical revert surface is wider than the original toggle scope.

5. Branch name still feat/studio-preview-pasteboard-bg — yesterday's note still applies, though diluted by the larger scope. Cosmetic.

CI

All required checks green on 414320a: Test, Typecheck, Lint, Build, Format, CodeQL, Test: runtime contract, CLI smoke (required), Smoke: global install, Preview parity, preview-regression, Tests on windows-latest, Render on windows-latest, regression-shards (hdr / styles-g), player-perf. One regression-shards (styles-e) still in-progress. mergeable_state: blocked is reviewer-gate.

Withdrawing my prior REQUEST_CHANGES. Re-approving on the new design.

— Re-review by Rames Jusso (pr-review)

[vanceingalls]

Re-review. Architecture pivot: the JSON manifest + opt-out toggle were removed entirely. Positions persist directly to HTML inline styles + CSS custom properties. This is the right call — the prior design's correctness gaps (4 blockers from my last review) were all rooted in the toggle being an additional gate that had to be enforced at every capability site. Removing the gate removes the surface.

Audited blockers from prior review

1. Required CI Test red on DomEditOverlay.test.ts:113 — ADDRESSED. canMove: true added to the test capabilities fixture in this PR (DomEditOverlay.test.ts:116). All required CI green on 414320a: Test, Tests on windows-latest, Typecheck, Lint, Build, CLI smoke, Test: runtime contract, regression-shards (render-compat / styles-a..g / hdr), Render on windows-latest, preview-regression, Player perf (load/fps/scrub/drift/parity), CodeQL. Two optional shards still in-progress (regression-shards (fast), regression-shards (styles-e)), non-blocking.
2. Multi-select drag bypasses toggle — NO LONGER APPLICABLE (no toggle). The drag site still got a defensive tightening though: DomEditOverlay.tsx:311 now gates on !groupCanMove in addition to !allowCanvasMovement, which is the right shape — multi-select drag only fires when every selected element passes the capability check.
3. Single-element resize/rotate handles render and fire for non-absolute when toggle off — NO LONGER APPLICABLE. Per-capability gates (canApplyManualOffset / canApplyManualSize / canApplyManualRotation) are now the single source of truth and they're tied to the element's actual structural properties.
4. Defence-in-depth gap at gesture boundary — NO LONGER APPLICABLE. startGesture checks the same capability flags; there's no second invariant to drift from.

Calibrated strengths

• sourcePatcher.ts:158-186 — the value: string | null semantics are cleanly threaded: existing-style branch deletes the prop, no-style branch is a no-op for null, and the rebuilt-style serialization is unchanged for the set branch. Empty style="" left harmless and noted.
• sourcePatcher.ts:228-238, 270-275 — both attribute paths add \\s+${escapeRegex(fullAttr)}= removal patterns, and the previously-bare attrPattern is now wrapped in escapeRegex (latent issue with data- attrs containing regex-meta chars also fixed in passing).
• manualEditsDom.ts:208-227 — stripGsapTranslateFromTransform is exactly the right shape: extract translate from the GSAP-managed matrix via DOMMatrix, write back without m41/m42, and drop the property entirely when the resulting matrix is identity. The DOMMatrix lookup is window-scoped so iframe contexts work.
• manualEditsRenderScript.ts:247-272 — the Object.defineProperty setter trap on __hf.seek / __player.renderSeek is the right fix for the Puppeteer macrotask-starvation regression. WRAPPED_PROP guard prevents double-wrap, and there's still a 6s polling fallback for non-configurable cases.
• htmlCompiler.ts:996-1008 — script injection is gated by attribute presence, so the runtime cost is zero for projects that don't use studio positioning.

Important (not blocking)

• No unit tests for the new value: null semantics in sourcePatcher. sourcePatcher.test.ts is untouched in this PR — none of its existing cases pass value: null, so the removal branches in patchInlineStyleInTag (lines 172-188), patchAttributeByTarget (lines 304-310), patchAttribute (lines 333-339), and the text-content null no-op (lines 423, 442) are entirely uncovered. This is the new architectural primitive of the whole PR — a single happy-path round-trip test (buildPathOffsetPatches -> applyPatchByTarget -> grep for --hf-studio-offset-x in result, then buildClearPathOffsetPatches -> applyPatchByTarget -> verify it's gone) would lock the contract.
• composeRotation silently drops authored axis-form rotates. manualEditsRenderScript.ts:152-157 falls back to return rotationValue when isSimpleRotateAngle rejects the original (e.g., 45deg X Y axis form, or var(...)). The studio path uses calc(original + studio) for the simple case and just studio for the complex case — which means authored 3D rotations get overwritten by studio rotations. Probably fine in practice (studio doesn't ship 3D rotation today) but worth a TODO comment so a future user of authored axis-rotates doesn't get a silent regression.
• Render-path injection idempotence. htmlCompiler.ts:1000-1008 appends <script>...</script> before the first </body>. If compileForRender is ever called on already-compiled output (or the runtime cache double-fires), you'd get two copies of the script in the HTML. The runtime is self-guarded against double-wrapping via WRAPPED_PROP, so this is benign at runtime — but a one-line if (htmlWithAssets.includes("__hfStudioPositionSeekReapplyWrapped")) skip would make the HTML output deterministic across recompiles.

Nits

• manualEditsDom.ts:215 — (win as unknown as { DOMMatrix?: typeof DOMMatrix })?.DOMMatrix is more cast-juggling than needed; the iframe window does have DOMMatrix typed natively. win?.DOMMatrix works.
• useDomEditCommits.ts:227-244 — commitPositionPatchToHtml swallows errors via .catch(showToast) but the inner queueDomEditSave already runs the body inside its own promise chain; if queueDomEditSave itself rejects synchronously (it doesn't today, but the type doesn't preclude it), the catch may not fire. Defensive try { void queueDomEditSave(...).catch(...) } catch { showToast(...) } would be more robust.
• The closed group-drag write loop at useDomEditCommits.ts:241-247 calls commitPositionPatchToHtml per-element with the same coalesce key — relies on the save queue to dedupe. Worth a comment.

Cited prior reviews

• @jrusso1020's initial APPROVE (pre-pivot) and follow-up COMMENT recalibrating after my prior REQUEST CHANGES are auto-invalidated by branch protection's "re-approval after push" rule; they're not in scope for this verdict.

---

Verdict: APPROVE
Reasoning: The pivot deletes the entire opt-out toggle surface that all 4 prior blockers attacked, and the new direct-to-HTML persistence is sound (clean null-removal contract in sourcePatcher, correct GSAP matrix strip, proper seek-trap for render). The test-coverage gap on the new value: null primitive is important but the production paths exercising it are integration-tested via the test plan and now green-on-CI; recommend a follow-up to lock the contract.

Review by Vai (re-review)