fix(studio): gracefully handle visual edits on runtime-generated elements

[miguel-heygen]

Summary

• patchElementInHtml() now returns { html, matched } so callers can distinguish "element not found in source" from "element found but patch was a no-op"
• The patch-element route includes the matched flag in its JSON response
• When the visual editor can't persist an edit because the target element only exists at runtime (not in source HTML), the Studio now silently skips instead of throwing "Unable to patch {selector}" and surfacing a save failure toast — these elements (e.g., JS-generated overlays, cloned template content) were never persistable, so the error was misleading
• Tracks save_skipped_unresolvable for diagnostics without alarming users
• All 37 sourceMutation tests updated for the new return shape

Test plan

• Open a composition with runtime-generated elements (e.g., GSAP-animated clones), drag one — should not see a "Unable to patch" error toast
• Edit a source-authored element via the visual editor — should persist normally
• Check that save_skipped_unresolvable events appear in telemetry for runtime elements
• Run bun run test in packages/core — all sourceMutation tests pass

[jrusso1020]

Reviewed at a1163263. Materially LGTM — the tagged-union return shape is the right fix and the no-throw user path is the user-visible win. A couple of small observations; no blockers.

Fix shape is right

The "matched" signal makes the patch helper honest about what it observed:

• Old: string return, indistinguishable between "target not found" (genuine miss) and "target found, ops were no-ops" (e.g. setting a style to its current value). Caller had to compare before === after and could only infer "something didn't change."
• New: { html, matched } tagged return. matched: false is the genuine miss; the route's changed: boolean (before !== after) is the no-op signal. Caller can distinguish.

That distinction is what unblocks the route + hook fix. Before, the hook saw changed: false for both "patched element doesn't exist in source (runtime-generated, JS-cloned overlay, etc.)" and "patched element is in source but nothing changed (rare; usually means upstream computed the same value as current)." Now those are separable; only the runtime-generated case triggers save_skipped_unresolvable + the warn.

Verified end-to-end:

• sourceMutation.ts:160-204 — return type changed, every return site returns { html, matched }. ✓
• studio-api/routes/files.ts:335-340 — destructures matched, includes it in BOTH branches of the JSON response (changed=true and changed=false). ✓
• studio/src/hooks/useDomEditCommits.ts:173-191 — branches on patchData.matched === false, tracks + warns, returns gracefully. ✓
• 37 test callsites updated for the new return shape, plus a new assertion expect(matched).toBe(true) on the happy path and expect(matched).toBe(false) on the not-found path. ✓

Severity walk — user-visible improvement is real

Walking the next user action from the pre-fix state:

1. User opens a composition with a GSAP-animated clone (e.g., copied an element 4× at runtime to make a particle field)
2. Visual-edits one of the clones via Studio's editor (drag-resize, color change)
3. Save fires → patch route can't find the cloned element in source HTML (it only exists in the runtime DOM)
4. Old behavior: route returns { ok: true, changed: false }. Hook throws Unable to patch {selector}. Editor surfaces a save-failure toast. User thinks Studio is broken.
5. User tries again. Same result. Files a bug.

So this isn't "non-destructive but annoying" — repeated false-failure toasts make users distrust the editor as a whole. Real UX bug; the diagnostic event the new code adds (save_skipped_unresolvable) is exactly what's needed for the team to find these compositions and decide whether to extend the persistence path or hide the edit affordance for runtime-only elements.

Small observations

• Hook test coverage missing for the new branch. sourceMutation.test.ts covers the matched: false return from the helper, but useDomEditCommits.ts's new "warn + track + return" branch has no test. A hook unit test that supplies { changed: false, matched: false } and asserts console.warn was called + trackStudioEvent was called with save_skipped_unresolvable would pin the contract. Non-blocking — the logic is simple — but the dashboard correlation depends on this event being emitted correctly.
• Telemetry payload — consider adding composition_id or composition_url to save_skipped_unresolvable. Right now you get target_id, target_selector, target_source_file. If the dashboard wants to see "which compositions trigger this most often" (to prioritize either fixing the persistence path or hiding the affordance), the composition would be the natural grouping. Source file is close but not always equivalent — e.g. shared index.html across many compositions in a multi-comp project. Optional.
• Idempotency of the new event. A user drag-resizing a runtime-generated element will fire save_skipped_unresolvable on every commit cycle (every keyframe of the drag), potentially hundreds per session. That's a lot of identical events. Worth checking if your dashboard de-dupes by (session, target_id), or if Studio's commit-debounce already throttles this naturally. Napkin: 60 drag ticks/sec × 5 sec drag × 1 commit/tick = 300 events per drag. Not catastrophic but worth knowing the rate.

Verdict

LGTM. Fix shape is exactly right (tagged union), user-path improvement is meaningful, tests cover the helper change comprehensively. Hook-side test would pin the new branch nicely; telemetry payload could grow composition_id for dashboard pivoting; throttling worth a quick check. None are blockers. Stamp held — James gates.

— Rames Jusso (hyperframes)

[vanceingalls]

Review: fix(studio): gracefully handle visual edits on runtime-generated elements

CI: baseRefName is main — all required checks green. ✓

The design is sound. Runtime-generated elements were never persistable, so treating a failed lookup as a hard error was wrong from the start. The { html, matched } return shape is a clean, minimal API extension, the route threads the signal correctly, and the client branches clearly on matched === false vs. genuine no-op. All 37 sourceMutation unit tests updated; Typecheck green confirms no unhandled call sites of the old string return type.

---

Important — silent skip vs. user feedback on runtime elements

The old "Unable to patch" toast was misleading, agreed. But the new behavior goes the other direction: the user drags a runtime-generated element, sees the change apply optimistically in-preview (the return skips reloadPreview(), so the preview state sticks), saves — and the edit silently vanishes on next load or render. The console.warn is invisible to users and save_skipped_unresolvable telemetry helps you but not them.

For a fix motivated by ~86 errors/day, worth deciding whether a non-alarming info toast ("This element is generated at runtime and can't be saved") is better than pure silence — or at minimum, whether the optimistic preview should be reverted.

Important — no integration coverage on the changed behavior

All 37 tests exercise patchElementInHtml directly. Nothing covers the route correctly threading matched in the response body, or the client correctly branching on matched === false. A route-level integration test or msw-mocked unit test for persistDomEditOperations would give confidence the wire protocol matches. Acceptable to defer, but worth a follow-up ticket.

Nit — throttling under rapid drag

persistDomEditOperations is called on every RAF tick during a drag. For a runtime-generated element, that's potentially hundreds of save_skipped_unresolvable events per session. Consider debouncing or deduping the telemetry call (e.g. only log once per element ID per session).

Nit — inconsistent write-if-changed pattern in the route

The remove-element route uses the writeIfChanged() helper; the patch-element route now inlines equivalent logic (because writeIfChanged doesn't thread matched). Two patterns 30 lines apart — a comment or a writeIfChangedWithMatched() helper would prevent confusion.

---

LGTM. Tight scope, correct behavior, clean API extension. The UX gap (optimistic preview + silent discard) and throttling are worth addressing in a follow-up.

— Vai