refactor(core): unify composition contract (#2157)

* refactor(core): unify composition contract

* fix(parsers): parse start expressions linearly

packages/studio-server/src/helpers/previewAdapter.ts

@@ -5,7 +5,7 @@ import {
   STUDIO_HEIGHT_PROP,
   STUDIO_MANUAL_EDIT_GESTURE_ATTR,
 } from "./draftMarkers.js";
-import { parseStartExpression } from "@hyperframes/core/runtime/start-expression";
+import { readClipTiming } from "@hyperframes/core/composition-contract";
 
 export type DraftPayload =
   | { type: "move"; hfId: string; dx: number; dy: number }
@@ -139,77 +139,38 @@ export function createPreviewAdapter(
     },
 
     getElementTimings() {
-      // data-start can be a relative-reference expression ("intro", "intro + 2" —
-      // parseStartExpression's grammar), not just an absolute number. A raw
-      // parseFloat on it (the old behavior) silently resolves any reference to
-      // undefined. Resolve references recursively against the target's own
-      // resolved end (data-end, or data-start + data-duration when the target
-      // has no data-end) — this function never read data-duration before either,
-      // so a reference to a duration-authored (not end-authored) clip used to be
-      // unresolvable regardless of the parseFloat bug.
-      //
-      // This is the third copy of "resolve relative data-start" (runtime
-      // startResolver.ts; the SDK's own getElementTimings in session.ts; this
-      // one). The runtime version is substantially more complex (host offsets,
-      // media, live timelines) so a shared extraction isn't a straightforward
-      // win — this one and the SDK's stay hand-kept in sync instead.
-      const startCache = new Map<Element, number | undefined>();
+      const timingCache = new Map<Element, { start: number | null; end: number | null }>();
       const visiting = new Set<Element>();
-
-      const resolveEnd = (el: Element): number | undefined => {
-        const endAttr = el.getAttribute("data-end");
-        if (endAttr !== null) {
-          const ev = parseFloat(endAttr);
-          if (Number.isFinite(ev)) return ev;
-        }
-        const durationAttr = el.getAttribute("data-duration");
-        const dv = durationAttr !== null ? parseFloat(durationAttr) : NaN;
-        const sv = resolveStart(el);
-        if (Number.isFinite(dv) && sv !== undefined) return sv + dv;
-        return undefined;
-      };
-
-      // Split out of resolveStart so its own branching stays low — mirrors the
-      // SDK's getElementTimings resolver split (resolveReferenceStart).
-      const resolveReferenceStart = (refId: string, offset: number): number | undefined => {
-        const target = findById(refId);
-        const targetEnd = target ? resolveEnd(target) : undefined;
-        return targetEnd !== undefined ? Math.max(0, targetEnd + offset) : undefined;
-      };
-
-      const resolveStart = (el: Element): number | undefined => {
-        if (startCache.has(el)) return startCache.get(el);
-        if (visiting.has(el)) return undefined; // reference cycle — fail safe, don't loop
+      const resolveTiming = (el: Element): { start: number | null; end: number | null } => {
+        const cached = timingCache.get(el);
+        if (cached) return cached;
+        if (visiting.has(el)) return { start: null, end: null };
         visiting.add(el);
-        let resolved: number | undefined;
         try {
-          const startStr = el.getAttribute("data-start");
-          const expr = parseStartExpression(startStr);
-          if (expr?.kind === "reference") {
-            resolved = resolveReferenceStart(expr.refId, expr.offset);
-          } else if (expr?.kind === "absolute") {
-            resolved = expr.value;
-          } else {
-            // parseStartExpression returns null for empty/absent data-start, and
-            // also for a malformed grammar string (e.g. "3 abc" — a leading
-            // number followed by content the reference regex rejects). The
-            // parseFloat below only ever succeeds on that second, malformed
-            // case (a clean number or a clean reference already matched above).
-            const sv = startStr !== null ? parseFloat(startStr) : NaN;
-            resolved = Number.isFinite(sv) ? sv : undefined;
-          }
+          const timing = readClipTiming(el, {
+            defaultStart: null,
+            resolveReferenceEnd: (refId) => {
+              const target = findById(refId);
+              return target ? resolveTiming(target).end : null;
+            },
+          });
+          const resolved = { start: timing.start, end: timing.end };
+          timingCache.set(el, resolved);
+          return resolved;
         } finally {
           visiting.delete(el);
         }
-        startCache.set(el, resolved);
-        return resolved;
       };
 
       const result: Record<string, { start?: number; end?: number }> = {};
       for (const el of doc.querySelectorAll("[data-hf-id]")) {
         const hfId = el.getAttribute("data-hf-id");
         if (!hfId) continue;
-        result[hfId] = { start: resolveStart(el), end: resolveEnd(el) };
+        const timing = resolveTiming(el);
+        result[hfId] = {
+          start: timing.start ?? undefined,
+          end: timing.end ?? undefined,
+        };
       }
       return result;
     },

packages/studio-server/src/helpers/sourceMutation.ts

@@ -3,6 +3,7 @@ import postcss from "postcss";
 import selectorParser from "postcss-selector-parser";
 import { isAllowedHtmlAttribute, isSafeAttributeValue } from "@hyperframes/core/html-attr-safety";
 import { ensureHfIds } from "@hyperframes/parsers/hf-ids";
+import { readClipTiming, writeClipTiming } from "@hyperframes/core/composition-contract";
 import { parseStyleDecls, patchStyleAttrString } from "./sourceStyleMutation.js";
 
 export interface SourceMutationTarget {
@@ -239,30 +240,16 @@ export interface SplitElementResult {
 function resolveElementTiming(el: Element): {
   start: number;
   duration: number;
-  usesDataEnd: boolean;
 } {
-  const start = parseFloat(el.getAttribute("data-start") ?? "0") || 0;
-  const usesDataEnd = el.hasAttribute("data-end");
-  const duration = usesDataEnd
-    ? parseFloat(el.getAttribute("data-end") ?? "") - start || 0
-    : parseFloat(el.getAttribute("data-duration") ?? "0") || 0;
-  return { start, duration, usesDataEnd };
+  const timing = readClipTiming(el);
+  return { start: timing.start ?? 0, duration: timing.duration ?? 0 };
 }
 
-function setElementDuration(
-  el: Element,
-  start: number,
-  duration: number,
-  usesDataEnd: boolean,
-): void {
-  if (usesDataEnd) {
-    const endTime = String(Math.round((start + duration) * 1000) / 1000);
-    el.setAttribute("data-end", endTime);
-    el.removeAttribute("data-duration");
-  } else {
-    el.setAttribute("data-duration", String(Math.round(duration * 1000) / 1000));
-    el.removeAttribute("data-end");
-  }
+function setElementDuration(el: Element, start: number, duration: number): void {
+  writeClipTiming(el, {
+    start: Math.round(start * 1000) / 1000,
+    duration: Math.round(duration * 1000) / 1000,
+  });
 }
 
 // fallow-ignore-next-line complexity
@@ -278,7 +265,6 @@ export function splitElementInHtml(
   if (!el || !isHTMLElement(el)) return { html: source, matched: false, newId: null };
 
   const timing = resolveElementTiming(el);
-  const { usesDataEnd } = timing;
   let { start, duration } = timing;
   // GSAP-animated elements carry their timing in the script, not in data-* attrs,
   // so the source has no authored duration. Fall back to the store's (GSAP-derived)
@@ -310,8 +296,7 @@ export function splitElementInHtml(
   // Descendants carry their own data-hf-id; leaving them duplicates the id of
   // every nested node (e.g. an inner <span>), so strip them on the clone too.
   for (const node of clone.querySelectorAll("[data-hf-id]")) node.removeAttribute("data-hf-id");
-  clone.setAttribute("data-start", String(Math.round(splitTime * 1000) / 1000));
-  setElementDuration(clone, splitTime, secondDuration, usesDataEnd);
+  setElementDuration(clone, splitTime, secondDuration);
 
   // Keep the "clip" class — the runtime uses it to control visibility
   // based on data-start/data-duration timing.
@@ -340,8 +325,7 @@ export function splitElementInHtml(
 
   // Trim the original element's duration. A GSAP element had no data-start; stamp
   // it so the runtime windows the first half (visibility selects on [data-start]).
-  el.setAttribute("data-start", String(Math.round(start * 1000) / 1000));
-  setElementDuration(el, start, firstDuration, usesDataEnd);
+  setElementDuration(el, start, firstDuration);
 
   // Insert clone after original
   if (el.nextSibling) {

packages/studio-server/src/helpers/sourceMutationSplitAndGroup.test.ts

@@ -32,6 +32,21 @@ describe("splitElementInHtml", () => {
     expect(result.html).toContain('data-duration="4"');
   });
 
+  it("canonicalizes legacy timing attributes on both split halves", () => {
+    const legacy = source.replace(
+      'data-start="1" data-duration="6"',
+      'data-start="1" data-end="7" data-layer="3"',
+    );
+    const result = splitElementInHtml(legacy, { id: "box" }, 3, "box-split");
+
+    expect(result.matched).toBe(true);
+    expect(result.html).not.toContain("data-end=");
+    expect(result.html).not.toContain("data-layer=");
+    expect(result.html.match(/data-track-index="3"/g)).toHaveLength(2);
+    expect(result.html).toContain('data-duration="2"');
+    expect(result.html).toContain('data-duration="4"');
+  });
+
   it("duplicates CSS rules for the new element ID", () => {
     const result = splitElementInHtml(source, { id: "box" }, 3, "box-split");
     expect(result.html).toContain("#box-split");