From 4a36655b2bd966ed81afcdcd2afb3bb201810b09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel?= <miguel.sierra@heygen.com>
Date: Tue, 7 Jul 2026 18:11:21 -0400
Subject: [PATCH] fix(engine): resolve relative data-start references in
 video-frame extraction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(engine): resolve relative data-start references in video-frame extraction

<video data-start="intro"> (a relative reference to another clip's end) is
resolved by the browser runtime but parseVideoElements/parseImageElements did a
raw parseFloat, yielding NaN start/end. The FrameLookupTable active-window
checks (start <= t <= end) are then always false, so the clip is never injected
and composites BLANK in the final render — while lint/validate/inspect/snapshot
and the live preview all look fine. The docs' Relative Timing section teaches
exactly this pattern on <video>.

Share the pure reference-syntax parser (parseStartExpression) out of the runtime
resolver into @hyperframes/core, and resolve references in the extractor against
the linkedom document it already holds: a reference resolves to the target
clip's resolved start + its duration (data-duration or data-end) + offset,
mirroring the runtime. Cycle-guarded; an unknown target or unknown duration
falls back to the target's start / 0 (never NaN), matching runtime semantics.
Natural-media-duration-only targets aren't known at parse time (same limit as
the runtime's fallback). parseImageElements gets the same fix.

Runtime resolver behavior is unchanged (its 25-case suite still passes).

* chore: re-trigger CI to refresh a stuck CodeQL aggregate check
---
 packages/core/src/index.ts                    |   5 +
 packages/core/src/runtime/startExpression.ts  |  47 ++++++++
 packages/core/src/runtime/startResolver.ts    |  37 +-----
 .../src/services/videoFrameExtractor.test.ts  |  83 +++++++++++++
 .../src/services/videoFrameExtractor.ts       | 113 +++++++++++++++++-
 5 files changed, 246 insertions(+), 39 deletions(-)
 create mode 100644 packages/core/src/runtime/startExpression.ts

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index 632aaa0d7..fec8f7652 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -232,6 +232,11 @@ export type { FitTextOptions, FitTextResult } from "./text/index.js";
 
 // Runtime helpers (composition-side)
 export { getVariables } from "./runtime/getVariables.js";
+export {
+  parseStartExpression,
+  parseNumeric,
+  type ReferenceExpression,
+} from "./runtime/startExpression.js";
 
 // Variable validation (CLI / tooling-side)
 export {
diff --git a/packages/core/src/runtime/startExpression.ts b/packages/core/src/runtime/startExpression.ts
new file mode 100644
index 000000000..4756595ef
--- /dev/null
+++ b/packages/core/src/runtime/startExpression.ts
@@ -0,0 +1,47 @@
+/**
+ * Pure parser for the `data-start` timing expression grammar, shared by the
+ * browser runtime resolver (`createRuntimeStartTimeResolver`) and the Node-side
+ * video-frame extractor (`parseVideoElements`) so both agree on exactly what a
+ * relative reference means. No DOM/browser dependencies — safe to import in
+ * Node.
+ *
+ * Grammar (matches the docs' "Relative Timing" section):
+ *   - `"12.5"`            -> absolute seconds
+ *   - `"intro"`           -> start when clip `intro` ends
+ *   - `"intro + 2"`       -> 2s after `intro` ends
+ *   - `"intro - 0.5"`     -> 0.5s before `intro` ends (overlap)
+ */
+
+export type ReferenceExpression =
+  | { kind: "absolute"; value: number }
+  | { kind: "reference"; refId: string; offset: number };
+
+/** Parse a value to a finite number, or `null` if it isn't one. */
+export function parseNumeric(value: string | null | undefined): number | null {
+  if (value == null || value === "") return null;
+  const parsed = Number(value);
+  return Number.isFinite(parsed) ? parsed : null;
+}
+
+/**
+ * Parse a raw `data-start` value into an absolute time or a clip reference.
+ * Returns `null` when the value is empty or not a recognized expression.
+ */
+export function parseStartExpression(raw: string | null | undefined): ReferenceExpression | null {
+  const normalized = (raw ?? "").trim();
+  if (!normalized) return null;
+  const absolute = parseNumeric(normalized);
+  if (absolute != null) {
+    return { kind: "absolute", value: absolute };
+  }
+  const referenceMatch = normalized.match(/^([A-Za-z0-9_.:-]+)(?:\s*([+-])\s*([0-9]*\.?[0-9]+))?$/);
+  if (!referenceMatch) return null;
+  const refId = (referenceMatch[1] ?? "").trim();
+  if (!refId) return null;
+  const sign = referenceMatch[2] ?? "+";
+  const offsetRaw = referenceMatch[3] ?? "0";
+  const parsedOffset = Number.parseFloat(offsetRaw);
+  const offsetMagnitude = Number.isFinite(parsedOffset) ? Math.max(0, parsedOffset) : 0;
+  const offset = sign === "-" ? -offsetMagnitude : offsetMagnitude;
+  return { kind: "reference", refId, offset };
+}
diff --git a/packages/core/src/runtime/startResolver.ts b/packages/core/src/runtime/startResolver.ts
index a797717f2..98246c76b 100644
--- a/packages/core/src/runtime/startResolver.ts
+++ b/packages/core/src/runtime/startResolver.ts
@@ -1,27 +1,11 @@
 import type { RuntimeTimelineLike } from "./types";
 import { swallow } from "./diagnostics";
 import { readElementPlaybackRate } from "./media";
+import { parseNumeric, parseStartExpression } from "./startExpression";
 
 const AUTHORED_DURATION_ATTR = "data-hf-authored-duration";
 const AUTHORED_END_ATTR = "data-hf-authored-end";
 
-type ReferenceExpression =
-  | {
-      kind: "absolute";
-      value: number;
-    }
-  | {
-      kind: "reference";
-      refId: string;
-      offset: number;
-    };
-
-function parseNumeric(value: string | null | undefined): number | null {
-  if (value == null || value === "") return null;
-  const parsed = Number(value);
-  return Number.isFinite(parsed) ? parsed : null;
-}
-
 function parseDurationAttr(element: Element): number | null {
   return parseNumeric(element.getAttribute("data-duration"));
 }
@@ -38,25 +22,6 @@ function parseAuthoredEndAttr(element: Element): number | null {
   return parseNumeric(element.getAttribute(AUTHORED_END_ATTR));
 }
 
-function parseStartExpression(raw: string | null | undefined): ReferenceExpression | null {
-  const normalized = (raw ?? "").trim();
-  if (!normalized) return null;
-  const absolute = parseNumeric(normalized);
-  if (absolute != null) {
-    return { kind: "absolute", value: absolute };
-  }
-  const referenceMatch = normalized.match(/^([A-Za-z0-9_.:-]+)(?:\s*([+-])\s*([0-9]*\.?[0-9]+))?$/);
-  if (!referenceMatch) return null;
-  const refId = (referenceMatch[1] ?? "").trim();
-  if (!refId) return null;
-  const sign = referenceMatch[2] ?? "+";
-  const offsetRaw = referenceMatch[3] ?? "0";
-  const parsedOffset = Number.parseFloat(offsetRaw);
-  const offsetMagnitude = Number.isFinite(parsedOffset) ? Math.max(0, parsedOffset) : 0;
-  const offset = sign === "-" ? -offsetMagnitude : offsetMagnitude;
-  return { kind: "reference", refId, offset };
-}
-
 export function createRuntimeStartTimeResolver(params: {
   timelineRegistry?: Record<string, RuntimeTimelineLike | undefined>;
   includeAuthoredTimingAttrs?: boolean;
diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts
index a31edcadf..103021e43 100644
--- a/packages/engine/src/services/videoFrameExtractor.test.ts
+++ b/packages/engine/src/services/videoFrameExtractor.test.ts
@@ -259,6 +259,74 @@ describe("parseVideoElements", () => {
       loop: true,
     });
   });
+
+  it("resolves a relative data-start reference to another clip's end", () => {
+    const videos = parseVideoElements(
+      '<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
+        '<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
+    );
+    const main = videos.find((v) => v.id === "main");
+    // intro ends at 10, so main starts at 10 and ends at 30 — not NaN.
+    expect(main?.start).toBe(10);
+    expect(main?.end).toBe(30);
+  });
+
+  it("applies + and - offsets on a relative reference", () => {
+    const videos = parseVideoElements(
+      '<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
+        '<video id="gap" src="b.mp4" data-start="intro + 2" data-duration="5"></video>' +
+        '<video id="overlap" src="c.mp4" data-start="intro - 0.5" data-duration="5"></video>',
+    );
+    expect(videos.find((v) => v.id === "gap")?.start).toBe(12);
+    expect(videos.find((v) => v.id === "overlap")?.start).toBe(9.5);
+  });
+
+  it("resolves chained references (A -> B -> C)", () => {
+    const videos = parseVideoElements(
+      '<video id="a" src="a.mp4" data-start="0" data-duration="4"></video>' +
+        '<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>' +
+        '<video id="c" src="c.mp4" data-start="b" data-duration="2"></video>',
+    );
+    expect(videos.find((v) => v.id === "b")?.start).toBe(4);
+    expect(videos.find((v) => v.id === "c")?.start).toBe(7); // 4 + 3
+  });
+
+  it("resolves a reference to a non-video timed element (div clip)", () => {
+    const videos = parseVideoElements(
+      '<div id="title" data-start="0" data-duration="6"></div>' +
+        '<video id="clip" src="b.mp4" data-start="title" data-duration="5"></video>',
+    );
+    expect(videos.find((v) => v.id === "clip")?.start).toBe(6);
+  });
+
+  it("derives a referenced clip's duration from data-end when data-duration is absent", () => {
+    const videos = parseVideoElements(
+      '<video id="intro" src="a.mp4" data-start="2" data-end="9"></video>' +
+        '<video id="main" src="b.mp4" data-start="intro" data-duration="5"></video>',
+    );
+    // intro: start 2, end 9 -> duration 7 -> main starts at 9.
+    expect(videos.find((v) => v.id === "main")?.start).toBe(9);
+  });
+
+  it("falls back to 0 (never NaN) for an unknown reference target", () => {
+    const videos = parseVideoElements(
+      '<video id="orphan" src="a.mp4" data-start="does-not-exist" data-duration="5"></video>',
+    );
+    const orphan = videos.find((v) => v.id === "orphan");
+    expect(orphan?.start).toBe(0);
+    expect(Number.isNaN(orphan?.start)).toBe(false);
+    expect(orphan?.end).toBe(5);
+  });
+
+  it("does not hang or NaN on a circular reference", () => {
+    const videos = parseVideoElements(
+      '<video id="a" src="a.mp4" data-start="b" data-duration="4"></video>' +
+        '<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>',
+    );
+    for (const v of videos) {
+      expect(Number.isNaN(v.start)).toBe(false);
+    }
+  });
 });
 
 describe("FrameLookupTable", () => {
@@ -334,6 +402,21 @@ describe("FrameLookupTable", () => {
     expect(table.getActiveFramePayloads(1.5).has("hero")).toBe(false);
   });
 
+  it("places a relative-reference video in its resolved window end-to-end (was blank)", () => {
+    // The reported bug: <video data-start="intro"> gave NaN start/end, so the
+    // active-window checks (start <= t <= end) were always false and the clip
+    // composited blank. With resolution, `main` is active across [10, 30].
+    const videos = parseVideoElements(
+      '<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
+        '<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
+    );
+    const table = createFrameLookupTable(videos, [{ ...fakeExtracted(600, 30), videoId: "main" }]);
+    expect(table.getActiveFramePayloads(5).has("main")).toBe(false); // before resolved start (10)
+    expect(table.getActiveFramePayloads(15).has("main")).toBe(true); // within [10, 30]
+    expect(table.getActiveFramePayloads(29).has("main")).toBe(true);
+    expect(table.getActiveFramePayloads(31).has("main")).toBe(false); // after resolved end (30)
+  });
+
   it("holds the last frame at the inclusive clip end (t === end)", () => {
     // clip [1,3] with exactly 2s of source frames (60 @ 30fps). The frame
     // landing on t === end used to deactivate one frame early and render blank,
diff --git a/packages/engine/src/services/videoFrameExtractor.ts b/packages/engine/src/services/videoFrameExtractor.ts
index 2c259b067..b42fb8071 100644
--- a/packages/engine/src/services/videoFrameExtractor.ts
+++ b/packages/engine/src/services/videoFrameExtractor.ts
@@ -10,7 +10,12 @@ import { spawn } from "child_process";
 import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs";
 import { isAbsolute, join, posix, resolve, sep } from "path";
 import { parseHTML } from "linkedom";
-import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
+import {
+  decodeUrlPathVariants,
+  MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
+  parseNumeric,
+  parseStartExpression,
+} from "@hyperframes/core";
 import { trackChildProcess } from "../utils/processTracker.js";
 import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
 import {
@@ -148,9 +153,102 @@ export interface ExtractionResult {
   phaseBreakdown: ExtractionPhaseBreakdown;
 }
 
+// Minimal structural DOM shape the reference resolver needs, so it works
+// against linkedom (Node) without pulling in lib.dom types.
+interface RefResolverEl {
+  getAttribute(name: string): string | null;
+}
+interface RefResolverDoc {
+  getElementById(id: string): RefResolverEl | null;
+  querySelector(selector: string): RefResolverEl | null;
+}
+
+/**
+ * Find the element a relative `data-start` reference points at — by `id`
+ * first, then by `data-composition-id` (a sub-composition can be referenced).
+ * The reference-id grammar (see parseStartExpression) is restricted to
+ * `[A-Za-z0-9_.:-]`, none of which need escaping inside a quoted attribute
+ * selector, so no CSS.escape (absent in linkedom) is required.
+ */
+function findReferenceTargetEl(doc: RefResolverDoc, refId: string): RefResolverEl | null {
+  return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`);
+}
+
+/**
+ * Resolve an element's absolute start time (seconds) the same way the browser
+ * runtime's startResolver does, so `<video data-start="intro">` (a relative
+ * reference to another clip's end) renders at the right time instead of
+ * producing NaN and compositing blank. Durations come from `data-duration` or
+ * `data-end` here; the natural-media-duration fallback isn't known at parse
+ * time, so — exactly like the runtime — an unknown-duration reference falls
+ * back to the target's start and an unknown target falls back to 0 (never NaN).
+ */
+function resolveReferencedStart(
+  doc: RefResolverDoc,
+  el: RefResolverEl,
+  startCache: Map<RefResolverEl, number>,
+  visiting: Set<RefResolverEl>,
+): number {
+  const cached = startCache.get(el);
+  if (cached !== undefined) return cached;
+  if (visiting.has(el)) return 0; // cycle guard (A -> B -> A)
+  visiting.add(el);
+  try {
+    const expression = parseStartExpression(el.getAttribute("data-start"));
+    if (!expression) {
+      startCache.set(el, 0);
+      return 0;
+    }
+    if (expression.kind === "absolute") {
+      const value = Math.max(0, expression.value);
+      startCache.set(el, value);
+      return value;
+    }
+    const target = findReferenceTargetEl(doc, expression.refId);
+    if (!target) {
+      startCache.set(el, 0);
+      return 0;
+    }
+    const targetStart = resolveReferencedStart(doc, target, startCache, visiting);
+    const targetDuration = resolveReferencedDuration(doc, target, startCache, visiting);
+    const resolved =
+      targetDuration != null && targetDuration > 0
+        ? Math.max(0, targetStart + targetDuration + expression.offset)
+        : Math.max(0, targetStart + expression.offset);
+    startCache.set(el, resolved);
+    return resolved;
+  } finally {
+    visiting.delete(el);
+  }
+}
+
+/**
+ * Duration of a referenced clip, from `data-duration` or `data-end - start`.
+ * Returns null when only the natural media duration would settle it (unknown
+ * at parse time) — the caller then treats the reference as duration-0.
+ */
+function resolveReferencedDuration(
+  doc: RefResolverDoc,
+  el: RefResolverEl,
+  startCache: Map<RefResolverEl, number>,
+  visiting: Set<RefResolverEl>,
+): number | null {
+  const durationAttr = parseNumeric(el.getAttribute("data-duration"));
+  if (durationAttr != null && durationAttr > 0) return durationAttr;
+  const endAttr = parseNumeric(el.getAttribute("data-end"));
+  if (endAttr != null) {
+    const start = resolveReferencedStart(doc, el, startCache, visiting);
+    const delta = endAttr - start;
+    if (Number.isFinite(delta) && delta > 0) return delta;
+  }
+  return null;
+}
+
 export function parseVideoElements(html: string): VideoElement[] {
   const videos: VideoElement[] = [];
   const { document } = parseHTML(unwrapTemplate(html));
+  const startCache = new Map<RefResolverEl, number>();
+  const visiting = new Set<RefResolverEl>();
 
   const videoEls = document.querySelectorAll("video[src]");
   let autoIdCounter = 0;
@@ -170,7 +268,12 @@ export function parseVideoElements(html: string): VideoElement[] {
     const mediaStartAttr = el.getAttribute("data-media-start");
     const hasAudioAttr = el.getAttribute("data-has-audio");
 
-    const start = startAttr ? parseFloat(startAttr) : 0;
+    // Resolve data-start, including relative references ("intro", "intro + 2")
+    // to another clip's end — the browser runtime resolves these but a raw
+    // parseFloat here would yield NaN, placing the clip at NaN so it composites
+    // blank in the final render. `startAttr` may be a plain number or a
+    // reference; the resolver handles both.
+    const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
     // Derive end from data-end → data-start+data-duration → Infinity (natural duration).
     // The caller (htmlCompiler) clamps Infinity to the composition's absoluteEnd.
     let end = 0;
@@ -206,6 +309,8 @@ export interface ImageElement {
 export function parseImageElements(html: string): ImageElement[] {
   const images: ImageElement[] = [];
   const { document } = parseHTML(unwrapTemplate(html));
+  const startCache = new Map<RefResolverEl, number>();
+  const visiting = new Set<RefResolverEl>();
 
   const imgEls = document.querySelectorAll("img[src]");
   let autoIdCounter = 0;
@@ -222,7 +327,9 @@ export function parseImageElements(html: string): ImageElement[] {
     const endAttr = el.getAttribute("data-end");
     const durationAttr = el.getAttribute("data-duration");
 
-    const start = startAttr ? parseFloat(startAttr) : 0;
+    // Resolve relative data-start references (see parseVideoElements) so a
+    // referenced image start doesn't become NaN and drop the image from the render.
+    const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
     let end = 0;
     if (endAttr) {
       end = parseFloat(endAttr);