feat(sdk): scaffold @hyperframes/sdk — engine layer (model, RFC 6902 patches, mutate, apply-patches)

[vanceingalls]

Summary

Scaffolds the @hyperframes/sdk engine layer — the pure, DOM-backed mutation core that all higher-level SDK operations build on.

• engine/model.ts — linkedom ParsedDocument, parseMutable, findById, getElementStyles, etc.
• engine/patches.ts — RFC 6902 path grammar and override-set key mapping (pathToKey / keyToPath)
• engine/mutate.ts — Phase 3a op handlers (setStyle, setText, setAttribute, setTiming, setHold, removeElement, setVariableValue, setCompositionMetadata); UnsupportedOpError for Phase 3b ops
• engine/apply-patches.ts — bounded RFC 6902 patch applier + applyOverrideSet for T3 embedded-mode init
• engine/serialize.ts — round-trip HTML serializer

Each handler mutates the live linkedom document and returns {forward, inverse} RFC 6902 patch pairs. No events emitted here — callers own event dispatch.

Test plan

• bun test packages/sdk — engine-layer unit tests pass
• bun run build — package builds cleanly

🤖 Generated with Claude Code

[vanceingalls]

• fix(sdk,core): css tokenizer, override-set replay, setattribute safety, persist errors #1350
• fix(studio): watch external project dirs so preview ETag invalidates #1347
• fix(studio,core): persist manual position edits for GSAP-owned elements #1346
• feat(lint): add gsap_studio_edit_blocked rule for manual timeline + GSAP element targeting #1345
• feat(sdk): session API, optional history + persist-queue, adapters — Phase 3a complete #1325
• feat(sdk): scaffold @hyperframes/sdk — engine layer (model, RFC 6902 patches, mutate, apply-patches) #1324 👈 (View in Graphite)
• feat(core): expose hf-ids as subpath export for @hyperframes/sdk #1323
• main

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

[miguel-heygen]

Solid foundation. Engine is well-structured, the RFC 6902 path grammar is clearly specified, and the 36-test mutate suite covers forward/inverse round-trips for every op. Two issues worth addressing before Phase 3b lands on top of this.

---

P2 — SdkDocument cached model is never retained; queries re-parse the whole DOM

CompositionImpl constructor receives _doc: SdkDocument (the underscore is the tell), but the value is discarded. This means getElements() / getElement() / find() — all three currently implemented as:

return flatElements(buildDocument(serializeDocument(this.parsed)).roots);

— run a full serialize → ensureHfIds → parseHTML → DOM walk on every call. In the headless example alone:

const textElements = comp.find({ tag: "div" });
// ...
const el = comp.getElement(id);  // ← second full re-parse

This is O(document_size) per query, and Phase 3b will add more callsites.

The SdkDocument snapshot built in openComposition() was clearly intended to be the cached view — it's just not being stored. The fix: keep it on the instance, mark it dirty on every dispatch/batch/applyPatches, and rebuild lazily on getElements(). Alternatively, walk the live linkedom DOM directly (the buildElement function already takes an Element; no need to serialize and reparse).

Also: the buildDocument(html) call in openComposition() fires ensureHfIds → parseHTML immediately after parseMutable(html) already did the same parse. The result is discarded. That's two cold parses on open, both wasted. Even if you fix the cache, you can deduplicate by using the same parseMutable result as the source.

P3 — parseStyleAttr duplicated in document.ts and model.ts

model.ts:parseStyleAttr and document.ts:parseInlineStyles do the same job with slightly different handling (camelCase conversion is in document.ts but not in model.ts, which uses its own toCamel). These will drift. Extract a shared parseStyleString(attr): Record<string, string> utility (camelCase output) into a shared module, or into core.

[james-russo-rames-d-jusso]

Reviewed at the head of this PR. Substantial scaffold — F1-F10 spec markers consistent throughout, RFC 6902 patch grammar well-defined, round-trip test discipline solid. Most of the concerns below are about spec drift in the docs or contract gaps that will bite SDK consumers later, not implementation bugs.

Strengths

• F6 anti-duplication is real. document.ts and engine/model.ts both build atop @hyperframes/core ensureHfIds. No re-implementation of the parser/ID-minting layer. Good.
• Linkedom IS the mutable backing store. Single source of truth, no parallel mutable tree to keep in sync with the DOM. Significantly simpler than the alternative.
• JSON Pointer escaping correctly implemented in attrPath (patches.ts:39) — ~ → ~0, / → ~1 per RFC 6902 §3. Inverse correctly applied in parsePath (apply-patches.ts:33).
• Round-trip test discipline. Every mutate handler has an "inverse patches restore original state" test that does the full mutate → applyPatchesToDocument(inverse) → equality. That's the right contract to pin and it's pinned.
• Bounded patch applier, not generic JSON Patch. apply-patches.ts deliberately handles only the path patterns mutate.ts emits. Smart — keeps the surface tight, no escape hatch for malformed paths.

Blockers

1. addGsapTween / setGsapTween / removeGsapTween are silently no-op on the public surface. Composition (types.ts:202-204) advertises them; CompositionImpl (session.ts:120-133) dispatches them; applyOp (mutate.ts:80-88) returns EMPTY for all Phase 3b op types; dispatch (session.ts:206-210) treats the empty result as a no-op, fires changeHandlers but no patch event. An SDK consumer building against the typed interface gets silent failures with no signal that anything's wrong. Fix options:

   • (a) Throw NotImplementedYet at the public method (preferred for pre-1.0; loud failure).
   • (b) Gate behind a experimental: true constructor option that activates the typed signatures only when enabled.
   • (c) Remove from the public Composition interface until Phase 3b lands.

   The README/PRD will (presumably) say these work. They don't.

2. unique symbol ORIGIN_APPLY_PATCHES doesn't cross realm boundaries (types.ts:135). Symbols can't be serialized through postMessage, can't survive JSON.stringify, and can't survive a worker/iframe boundary. The SDK is explicitly intended for embedded (T3) use, which by definition crosses these boundaries. If a host running in a parent frame receives a patch event over postMessage, event.origin === ORIGIN_APPLY_PATCHES will never match because the symbol identity is realm-local.

   Fix: change to a namespaced string sentinel: export const ORIGIN_APPLY_PATCHES = "@hyperframes/sdk:applyPatches" as const;. Lose the type-uniqueness guarantee but gain cross-realm portability, which is the more important property for an SDK.

3. moveElement writes left/top CSS (mutate.ts:67-71). HF compositions don't position via left/top — they use data-x/data-y data attributes consumed by GSAP or the runtime's positioning layer. Writing left: 100px; top: 50px will not move an element in any HF runtime I've seen. This op may be a placeholder, but the public method is exposed and tested (mutate.test.ts:332) as if it's load-bearing.

4. Composition width/height storage mismatches the HF runtime convention. SDK reads (document.ts:128-134) and writes (mutate.ts:280-298) width/height from/to the root element's inline CSS (style="width: 1920px"). The HF runtime convention is data-width="1920" / data-height="1080" attributes on the root (e.g. confirmed in init.ts runtime setup + #1319's test fixtures). Output from the SDK will not be picked up by the HF runtime as a valid composition. Either:

   • (a) Switch SDK to read/write data-width/data-height, OR
   • (b) Have the SDK write BOTH inline CSS and data-width/data-height for compatibility, OR
   • (c) Document this explicitly as an interop boundary the host must reconcile.

   (a) seems easiest and unambiguous.

Concerns

5. "RFC 6902 frozen contract" is overstated. JsonPatchOp (types.ts:108-112) implements add | remove | replace — three of RFC 6902's six operations. move, copy, test are absent. That's fine as a subset for emission, but the PR title and the F2 spec marker call it "RFC 6902 frozen contract" — which implies the full grammar. Either:

   • Document the subset explicitly: // Emit-only subset of RFC 6902. We never emit move/copy/test.
   • Or implement the missing three operations.

   For an SDK whose contract is "host receives JSON Patch documents," consumers reasonably expect the full RFC 6902 op set on inbound applyPatches(). If a host's external undo tool emits a move patch, applyPatchesToDocument silently drops it (apply-patches.ts:62).

6. findRoot and extractDimensions disagree on root priority.

   • model.ts:37-44: [data-hf-root] > #stage > body.firstElementChild
   • document.ts:126: #stage > [data-hf-root]

   For a document with both <div id="stage"> and <div data-hf-root>, the SDK's "find the root for mutations" returns one element and "find the root for dimensions" returns the other. Unify the priority. Suggest extracting a single shared findRoot(doc) helper.

7. document.ts ownText trims; model.ts getOwnText does NOT. Text mutation round-trip: buildDocument reports trimmed text; setText writes verbatim text; subsequent buildDocument reports trimmed again. If a consumer reads el.text, mutates with the same value, the persisted state has untrimmed text but the consumer sees trimmed. Pick one and use it consistently — probably untrimmed (preserves the user's whitespace if they wanted it).

8. Path grammar comment in patches.ts:8 is wrong. Says timing/{start|duration|trackIndex}. Actual implementation emits timing/start, timing/end, timing/trackIndex (duration is converted to end via start + duration). Update the comment to match the code, or change the patch grammar to use duration (less work to recompute on apply but matches the EditOp shape).

9. SdkDocument.html is a stale snapshot. types.ts:32 describes it as "ensureHfIds-stamped HTML used as the source of truth for serialization" but in buildDocument (document.ts:177) it's set to the BUILD-TIME HTML and never updated. After mutations, consumers reading doc.html get the original HTML, not the current state. Either drop this field (use serialize() always) or document explicitly that it's the build-time snapshot.

Nits

• linkedom@^0.18.12 in deps — pinned to a current version. Check that this matches the version core uses, to avoid two linkedom copies in the workspace.
• packages/sdk version 0.6.86 in package.json:3 — same as the current player. Pinning to the player version implies the two are locked. For a Phase 3a scaffold, starting at 0.0.1 or 0.1.0 would surface the SDK's own maturity vs the player's. Worth discussing with the stakeholders before publishing.
• sideEffects: false in package.json:15 — good for tree-shaking. Verify no hidden side effects (e.g. a top-level const subscriber = ... setting up global state). Quick scan looks clean.
• vitest.config.ts is 8 lines — what's in there? If it's just defineConfig({}), consider omitting and using the default — fewer config files per package.
• mutate.ts handleSetTiming at line 184-185 mixes ?? semantics: timing.start ?? oldStart — if oldStart is null AND timing.start is undefined, you skip the el.setAttribute("data-start", ...) write (line 187 guards) but newStart is also null so newDuration computation falls through correctly. Edge cases are handled, but the logic took a couple reads. A comment or named intermediate would help.
• apply-patches.ts applyOne's sibling-index restoration at line 145-147 uses children[v.siblingIndex] ?? null. If the parent has fewer children at apply-time than at snapshot-time (other elements deleted concurrently), the restored element silently inserts at end. Probably OK for the undo case; worth a one-line comment.

What I verified

• File-by-file walk of model.ts, patches.ts, apply-patches.ts, mutate.ts, document.ts, serialize.ts, types.ts, adapters/types.ts, index.ts, package.json.
• RFC 6902 §3 JSON Pointer escape rules against attrPath / parsePath prop escape handling.
• F1-F10 spec markers throughout the code map to coherent surface areas.
• Round-trip pattern in mutate.test.ts for every implemented op.
• ensureHfIds (packages/core/src/parsers/hfIds.ts) is content-keyed and deterministic (FNV-1a, no random) — so calling it twice on the same HTML produces identical IDs. This means the "double ensureHfIds in openComposition (session.ts:376-377)" is wasteful but not a correctness bug.

What I didn't verify

• Notion PRD + Shape Review docs (auth-gated; I read the F1-F10 spec markers in the code as proxies).
• Whether the F1 "selection" semantics match what UI hosts will need (the proxy pattern looks right but I didn't trace an end-to-end UI flow).
• Phase 3b parser-backed op design — currently stubs, will be reviewed when they land.

Solid scaffold. Blockers 1-4 are real and worth fixing before this lands on consumers; the rest are doc/contract polish. Pinging Vance for the next round.

— Review by Rames D Jusso

[vanceingalls]

Both findings addressed in cd85c20 on #1325 (the flagged code — document.ts, session.ts, openComposition — lives in that PR's diff, not this one):

• P2 re-parse: query API now walks the live linkedom DOM via a new buildRoots(document) and caches the snapshot, invalidated on dispatch/applyPatches. openComposition parses once (parseMutable); the discarded _doc param and redundant buildDocument call are gone.
• P3 style-parse dup: document.ts now reuses model.ts's getElementStyles — parseInlineStyles deleted. Side benefit: the model version guards --custom-prop names from camelCase mangling, which the deleted copy didn't.

No changes needed in this PR's files.

[miguel-heygen]

Building on Rames's review. Both fix commits since my last pass are housekeeping (TS noUncheckedIndexedAccess guard on children[i]?.nodeType, fallow suppressions, index.ts stack-isolation trim — then reverted in #1325). None of the blockers from either review are addressed yet.

✅ Fixed since last review

• setOwnText: children[i]?.nodeType optional-chain — correct fix for noUncheckedIndexedAccess
• index.ts/fallow: stacked-PR housekeeping, no semantic change

Still open — my findings

• P2 (re-parse on every query): confirmed still present. This resolves once _doc is retained and getElements()/find() walk the live linkedom DOM.
• P3 (parseStyleAttr vs parseInlineStyles duplication): still two separate implementations.

Amplifying from Rames

Blocker 3 + 4 are the ones I'd flag as the highest risk before consumers land on top. Both are silent correctness failures — the operation appears to succeed, the model mutates, but the HF runtime ignores the output:

• Blocker 3 — moveElement writes left/top CSS (mutate.ts:67-71): HF elements are positioned via data-x/data-y consumed by GSAP/the positioning layer. left: 100px; top: 50px will not move anything in the runtime. This is tested and exported as load-bearing, so Phase 3b code will build on a no-op. Fix before this lands on consumers.

• Blocker 4 — width/height via inline CSS vs data-width/data-height: document.ts:128-134 reads from style= and mutate.ts:280-298 writes to style=. HF runtime reads data-width/data-height. Fix (a) from Rames — switch SDK to data-* attributes — is the right call; it matches the runtime convention without a host-side reconciliation step.

Blocker 2 (ORIGIN_APPLY_PATCHES as unique symbol) is also worth escalating: the SDK's T3 embedded mode inherently crosses iframe/postMessage boundaries, where symbol identity breaks down. A namespaced string sentinel ("@hyperframes/sdk:applyPatches") is the correct fix — it's a one-liner change with a meaningful behavioral difference for the embedded use case.

[james-russo-rames-d-jusso]

Re-reviewed at 38778d5e (delta from 049cff6a — 4 files, +5/-12). Round-2 narrowed this PR's surface to the engine-layer-only contract (moved consumer-facing buildDocument/openComposition/createHistory/createPersistQueue exports to #1325 where the implementations live) and added two fallow-ignore comments for symbols that #1325 consumes. Plus an optional-chain safety on model.ts:119.

The trim makes the stack reviewable as discrete layers — clean. But none of the four round-1 blockers I flagged were addressed in this round, and three of them apply to the type surface that lives here (not the implementation in #1325).

Cleared

✅ children[i]?.nodeType in setOwnText (model.ts:119) — defensive guard against the (likely-unreachable) case where the children array was mutated mid-iteration.
✅ index.ts trimmed to engine-layer + type exports — Composition interface and EditOp union are still type-exported, but the value exports (buildDocument, openComposition, etc.) move with their implementations to #1325.

Still open from round-1

The four round-1 blockers all reproduce on the same SHAs in the round-2 head:

1. addGsapTween / setGsapTween / removeGsapTween silent no-op on the public type surface. types.ts:202-204 still advertises them in Composition; mutate.ts:80-88 still returns EMPTY for the Phase 3b op types. The CompositionImpl in #1325 will silently swallow these calls. Worth either gating Phase 3b methods behind a non-typed experimental: true opt-in until they ship, or moving them out of the public Composition surface for this PR series.

2. ORIGIN_APPLY_PATCHES: unique symbol (types.ts:135) is realm-local. Doesn't survive postMessage, structured-clone, or JSON.stringify. T3 embedded use is the explicit target for this SDK; the symbol identity check on the host side won't match across the iframe boundary. Switch to a namespaced string sentinel ("@hyperframes/sdk:applyPatches").

3. moveElement writes left/top CSS (mutate.ts:67-71). HF compositions don't use left/top for positioning — the runtime/GSAP convention is data-x/data-y or GSAP transform set. This op as currently emitted won't actually move elements in any HF runtime.

4. Composition width/height stored as inline CSS, not data-width/data-height. Confirmed against the HF runtime convention in init.ts (which I just reviewed via #1319 — root setup uses data-width="1920"/data-height="1080" attributes). The SDK's output won't be runtime-readable as a valid composition.

What I verified

• Round-2 diff against round-1: 4 files, +5/-12 — the round-2 changes are non-overlapping with the four blockers above.
• Engine-layer surface (model/patches/mutate/serialize/apply-patches/types/adapters/types) still passes my round-1 round-trip + RFC-6902-escape checks.
• Composition interface still includes the Phase 3b methods.
• ORIGIN_APPLY_PATCHES declaration unchanged.

Round-2 is a clean stacking improvement, no regressions. But the four blockers remain — and the deeper they get embedded in the public surface, the more painful they are to revert post-1.0. If any of these have a deliberate deferral story (e.g. "Phase 3b will land the GSAP ops with implementation and tests"), worth surfacing in the PR body so reviewers don't re-flag them every round.

— Review by Rames D Jusso

[vanceingalls]

Round-3 pushed (30f7c4f + 8c210c32). Status against the four blockers:

Fixed:

• B1 GSAP silent no-op (30f7c4f): applyOp now throws UnsupportedOpError (stable code E_UNSUPPORTED_OP) for all 9 Phase 3b ops; can() returns false so callers feature-detect. Examples updated to can()-guard the GSAP calls; error class re-exported from the package entry. Honest failure over silent success — option (a).
• B2 origin symbol (8c210c32): ORIGIN_APPLY_PATCHES is now the namespaced string "@hyperframes/sdk:applyPatches". Agree the cross-realm property outweighs type-uniqueness for an embedded SDK.
• B4 width/height (8c210c32): confirmed against init.ts applyCompositionSizing — data-width/data-height are a forced override applied only when present. Fix: style is always written; the data-* attr is updated when the composition already carries it (so the edit isn't clobbered on load); absent attrs stay absent so inverse patches stay exact. Mirrored in the patch applier, 3 new tests. extractDimensions now prefers data-* over inline style.
• B5 JsonPatchOp documented as the emit-only RFC 6902 subset; applier header notes move/copy/test are ignored.
• B8 path-grammar comment fixed to timing/{start|end|trackIndex}.
• B9 SdkDocument.html documented as a build-time snapshot.
• B6/B7 fixed on feat(sdk): session API, optional history + persist-queue, adapters — Phase 3a complete #1325 (where document.ts lives): root resolution unified on the engine's findRoot; ownText trim behavior documented.

Rebuttals:

• B3 moveElement left/top: this matches Studio's own commit convention — sourcePatcher emits inline-style patches with left/top (see sourcePatcher.test.ts:66, studioHelpers.ts:87 which classifies left/top/width/height as positional inline styles), and the generator doc states all elements are absolutely positioned relative to #stage. The data-x/data-y you found is the generator clip-model input (htmlParser.ts:222 parses it into transforms when generating compositions), not the runtime edit path. P0 parity = do what Studio does, which is left/top.
• Version 0.6.86: repo policy — all packages share one version (release:prepare bumps every manifest; publish.yml is version-locked). Diverging the SDK would break that pipeline. Maturity is signaled by it not being in publish.yml yet.
• linkedom dual-copy: core doesn't depend on linkedom directly — no duplicate-copy risk.

Also since your review, the adversarial pass found and we fixed two more: root build pipeline now includes the sdk package (package.json filter), and openComposition({overrides}) actually replays the override-set onto the base (it was stored but never applied) — both with tests.

[miguel-heygen]

Building on Rames's reviews. Three fix commits landed since round 2 (96f35d28, b6cdfff5, 2ca04a7b) — all four blockers are now resolved.

✅ Fixed

Blocker 1 — Phase 3b silent no-op (96f35d28)
applyOp now throws UnsupportedOpError (code: "E_UNSUPPORTED_OP") with a message pointing at can(). validateOp returns false for PHASE3B_OPS entries via !PHASE3B_OPS.has(op.type). Two tests pin both contracts. Clean.

Blocker 2 — ORIGIN_APPLY_PATCHES unique symbol (b6cdfff5)
types.ts now exports "@hyperframes/sdk:applyPatches" as const with JSDoc explaining the cross-realm motivation. Correct fix.

Blocker 3 — moveElement writing left/top CSS (2ca04a7b)
handleMoveElement() delegates to handleSetAttribute for data-x/data-y. Test updated; inverse round-trip test added. Matches HF positioning convention.

Blocker 4 — width/height dual-channel (b6cdfff5)
handleSetCompositionMetadata reads data-width/data-height first (source of truth when present), writes back to the attribute only if the composition already carries it, always writes inline style. Three new tests cover: both channels updated, inverse restores both, no data-* minting on style-only compositions. apply-patches.ts mirrors the same dual-channel logic.

Concerns 5, 8, 9 (Rames) — Also addressed: JsonPatchOp JSDoc documents the emit-only subset; patches.ts comment corrected to {start|end|trackIndex}; SdkDocument.html JSDoc clarified as build-time snapshot.

Still open (minor, no action needed in this PR)

• P2 re-parse on every query — elementsCache added in #1325 addresses this at the session layer. Nothing to do here.
• P3 parseStyleAttr duplication — still two implementations. Acceptable to defer until Phase 3b solidifies the style-mutation API.
• Concern 7 (ownText trimming) — documented as "trimmed display value" in JSDoc rather than fixed. Acceptable documented contract.
• Concern 6 (findRoot priority) — extractDimensions/extractDuration now delegate to findRoot() in #1325.

All four blockers cleared, tests pin the contracts.

The base branch was changed.