feat(core): applyPositionEdits gains force option and undo reset path

[vanceingalls]

Summary

Part 3/4 of the SDK gap-closure stack. Extends applyPositionEdits with an optional force flag and restores the captured pre-edit translate when an edit channel is removed during undo.

Why

Hosts otherwise need a per-element wrapper to force reapplication after an external translate clobber and to clean up the inline translate left behind by an undone position edit.

Implementation

• Forward { force?: boolean } to applyPositionEditToElement.
• Run a reset pass for orphaned original-translate markers before applying active edits.
• Clear reset bookkeeping so redo captures a clean baseline.
• Regenerate position-edits-render-inline.ts.

Validation

• src/runtime/positionEdits.test.ts: 22 tests passing, including force, reset, and redo coverage.
• Full core Vitest suite passing.
• Position-edit render artifact regenerated and self-tests passing.
• oxlint and oxfmt passing.

[vanceingalls]

• feat(core): export runtime start resolver with documentRef parameter #2502
• feat(core): applyPositionEdits gains force option and undo reset path #2501 👈 (View in Graphite)
• fix(sdk): attachSync re-syncs the override snapshot on iframe load #2500
• feat(sdk): getVariableValue({ base: true }) reads pre-override declared defaults #2499
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Review — #2501 feat(core): applyPositionEdits gains force option and undo reset path

Verdict: LGTM

SSOT check

applyPositionEdits is the single orchestrator of both apply and reset. The reset path reuses the same EDIT_ORIGINAL_TRANSLATE_ATTR marker the apply path captures — no separate state channel. The reset selector ([data-hf-edit-original-translate]:not([data-hf-edit-base-x]):not([data-hf-edit-base-y])) correctly identifies elements whose edit was undone (marker present, base attrs removed).

Reset path correctly:

• Restores the captured original translate (or removes inline translate if the original was empty)
• Clears the marker attribute
• Deletes the lastAppliedTranslate WeakMap entry (so a subsequent redo re-captures cleanly)

The force option bypasses the fold-guard that normally skips re-application when the translate was externally clobbered — necessary for redo after reset, since the guard bookkeeping no longer matches.

Tests: 4 cases (force over clobber, undo reset, empty-original removal, redo-after-reset). The auto-generated IIFE is updated to match. No blockers.

[james-russo-rames-d-jusso]

Reviewed at 049f72d4. packages/core has noUncheckedIndexedAccess: true and CI's Preview parity / Perf: parity jobs are red on TS2345 at positionEdits.ts:188 and :203 — the added inline sites can't compile. Two inline anchors below. Layering on Miga's LGTM: I disagree — the compile break is what the preview-regression failure resolves to; that CI's the surface, this is the root cause.

Blockers

🟠 packages/core typecheck fails on both new isStylable(el) call sites at positionEdits.ts:188 + :203.

error TS2345: Argument of type 'Element | undefined' is not assignable to parameter of type 'Element'.

The orphaned[i] / marked[i] reads return Element | undefined under noUncheckedIndexedAccess, but the new predicate is typed (el: Element) => el is HTMLElement. This kills every red check on the PR (Preview parity, preview-regression, Perf: parity) — they all block on the workspace build. Cheapest fix: widen the predicate to accept Element | undefined, e.g.

const isStylable = (el: Element | undefined): el is HTMLElement => {
  if (!el) return false;
  return RealmHTMLElement
    ? el instanceof RealmHTMLElement
    : typeof (el as HTMLElement).style?.setProperty === "function";
};

Both call sites then work unchanged. Alternative: null-guard at each site (if (!el || !isStylable(el)) continue;). Either lands the same behavior — pick whichever reads cleaner.

Fixing this should turn the parity checks green (they're gated on the workspace build compiling); if any parity failures remain post-fix that's a separate signal worth looking at.

Concerns

(none)

Nits

(none)

Green notes

🟢 Reset selector is precise. [EDIT_ORIGINAL_TRANSLATE_ATTR]:not([EDIT_BASE_X_ATTR]):not([EDIT_BASE_Y_ATTR]) only matches orphaned pre-edit markers — an element with a live edit still has BOTH the marker AND the base attrs, so reset is a no-op on the seek-driven re-apply hot path.

🟢 Reset restore contract is exhaustive. original === "" (element had no inline translate before edit) → removeProperty("translate"); non-empty → setProperty("translate", original). Both paths clear the marker and evict from lastAppliedTranslate so a redo re-captures a clean baseline. Pinned by three focused tests (restores captured pre-edit translate, removes inline translate entirely when captured original was none, redo after reset re-captures a clean baseline).

🟢 force opt-in shape. New callers pass { force: true }; existing sites (init.ts:89, :1246, positionEdits.ts:227, stubs/position-edits-render-entry.ts:12, the IIFE's seek re-apply loop) pass nothing and keep the fold-guard behavior. No unintended widening of "unconditional overwrite" semantics.

🟢 IIFE regeneration matches the module logic. The auto-generated position-edits-render-inline.ts mirrors the reset-then-apply flow with the same selector, the same .get(w) ?? "" empty-string branch, and the same S.delete(u) eviction. The transpiled IIFE also compiles (the minified form doesn't have the noUncheckedIndexedAccess narrowing constraint), so once the module fix lands and regenerates, the IIFE stays consistent.

What I didn't verify

• Whether the fold-guard bookkeeping is safe to force ALSO from the seek loop under some adversarial mid-play undo/redo. The current shape (seek doesn't force) is defensible; the IIFE would need a separate change if that ever needs to shift.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 Compile break — TS2345 (blocker). orphaned[i] returns Element | undefined under noUncheckedIndexedAccess (set in packages/core/tsconfig.json), but isStylable here is typed (el: Element) => el is HTMLElement. This kills the workspace build, which cascades into the three red parity/perf checks on the PR.

Cleanest fix: widen the predicate signature (top of the function block at :178) to (el: Element | undefined): el is HTMLElement with a leading if (!el) return false; — both this call site and the sibling at :203 then work unchanged. Alternative: null-guard here inline (if (!el || !isStylable(el)) continue;).

— Rames D Jusso

[james-russo-rames-d-jusso]

🟠 Same TS2345 as :188 — marked[i] is Element | undefined; predicate takes Element. Same fix (widen predicate signature at :178 to Element | undefined, or null-guard here inline) resolves both sites in one shot.

— Rames D Jusso

[james-russo-rames-d-jusso]

Post-merge delta audit 049f72d4d9…4ac7b4fa82 (R1 was reviewed pre-merge; single-commit follow-up landed at merge time).

Delta

Single commit 4ac7b4fa82 — fix(core): guard undefined NodeList index in applyPositionEdits loops. Two files, +3/-3.

• packages/core/src/runtime/positionEdits.ts (source) — both for loops over orphaned / marked NodeList results now guard el === undefined before isStylable(el).
  • Line 187-188: if (el === undefined || !isStylable(el)) continue;
  • Line 202-203: if (el === undefined || !isStylable(el)) continue;
• packages/core/src/generated/position-edits-render-inline.ts (generated IIFE) — regenerated with the matching guard: if(a===void 0||!r(a))continue; in both loops. Minifier reassigned identifiers (i→a, u shuffle) but functionally identical to the source patch.

Assessment

🟢 Guard is defensive against sparse or out-of-range indexed access on the NodeList result. In practice querySelectorAll returns a live NodeList that shouldn't have undefined at valid indices, but a) --noUncheckedIndexedAccess demands the check anyway, and b) TypeScript compilers with strict-index emit the check either way — this makes the runtime match the typed expectation.

🟢 Generated file is in sync with source. No hand-edit hazard.

🟢 No behavioral change on the happy path — the extra el === undefined branch is unreachable when querySelectorAll returns dense results.

Concerns

None — this is a mechanical guard-tightening. LGTM post-merge.

— Review by Rames D Jusso