feat(core): clip-model hf- ids minted at parse, emitted as data-hf-id (R1) (#1270)

* feat(core): clip-model hf- ids minted at parse, emitted as data-hf-id (R1)

* docs(core): document legacy-id round-trip in clip-model readback (R1 review)

Addresses Rames' review on #1270: clarifies that a pre-R1 clip authored with
id="my-title" round-trips as data-hf-id="my-title" (non-hf-shaped but stable,
exact-match) by design — targeting uses exact [data-hf-id="…"] match and does
not require the hf- shape; legacy values re-mint only at the R7 write-back. Not
a bug. Comment-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(core): fix misleading legacy-id migration comment in htmlParser.ts

The original comment said legacy data-hf-id values "are re-minted only
once the R7 write-back persists freshly-minted ids to source" — which is
incorrect. ensureHfIds skips elements that already carry data-hf-id, so
legacy values (e.g. data-hf-id="my-title") persist indefinitely and are
NOT automatically re-minted. Exact-match targeting still works correctly.
Update comment to reflect actual behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(core): update htmlParser tests for R1 hf- id minting

Pre-R1 tests expected clip ids to reflect legacy `id=` attributes.
After R1, ensureHfIds runs first and mints data-hf-id — so clip.id
reflects the minted hf- value unless the element already has data-hf-id.

Fix: add explicit data-hf-id to test HTML elements where tests assert
specific id values. Update no-id test to expect hf- format (/^hf-[a-z0-9]{4}$/)
instead of the pre-R1 generated-id fallback (/^element-\d+$/).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-06-09 00:19:31 -07:00
committed by GitHub
co-authored by Claude Sonnet 4.6
parent c12854b47c
commit 8d6b7cf515
4 changed files with 28 additions and 16 deletions
@@ -447,6 +447,7 @@ function generateZoomGsapAnimations(
function generateElementHtml(element: TimelineElement, keyframes?: Keyframe[]): string {
const baseAttrs = [
`id="${element.id}"`,
`data-hf-id="${element.id}"`,
`data-start="${element.startTime}"`,
`data-end="${element.startTime + element.duration}"`,
`data-layer="${element.zIndex}"`,
+8 -8
View File
@@ -17,8 +17,8 @@ describe("parseHtml", () => {
<html>
<body>
<div id="stage">
<div id="text1" data-start="0" data-end="5" data-name="Title"><div>Hello World</div></div>
<div id="text2" data-start="2" data-end="7" data-name="Subtitle"><div>Sub</div></div>
<div id="text1" data-hf-id="text1" data-start="0" data-end="5" data-name="Title"><div>Hello World</div></div>
<div id="text2" data-hf-id="text2" data-start="2" data-end="7" data-name="Subtitle"><div>Sub</div></div>
</div>
</body>
</html>
@@ -42,7 +42,7 @@ describe("parseHtml", () => {
<html>
<body>
<div id="stage">
<div id="comp1" data-start="0" data-end="10" data-type="composition" data-composition-id="abc123">
<div id="comp1" data-hf-id="comp1" data-start="0" data-end="10" data-type="composition" data-composition-id="abc123">
<iframe src="/compositions/abc123"></iframe>
</div>
</div>
@@ -65,9 +65,9 @@ describe("parseHtml", () => {
<html>
<body>
<div id="stage">
<video id="vid1" data-start="0" data-end="10" src="video.mp4" data-name="My Video"></video>
<audio id="aud1" data-start="0" data-end="5" src="music.mp3" data-name="Music"></audio>
<img id="img1" data-start="2" data-end="8" src="photo.jpg" data-name="Photo" />
<video id="vid1" data-hf-id="vid1" data-start="0" data-end="10" src="video.mp4" data-name="My Video"></video>
<audio id="aud1" data-hf-id="aud1" data-start="0" data-end="5" src="music.mp3" data-name="Music"></audio>
<img id="img1" data-hf-id="img1" data-start="2" data-end="8" src="photo.jpg" data-name="Photo" />
</div>
</body>
</html>
@@ -123,7 +123,7 @@ describe("parseHtml", () => {
const result = parseHtml(html);
expect(result.elements).toHaveLength(1);
expect(result.elements[0].id).toMatch(/^element-\d+$/);
expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/);
});
it("extracts GSAP script from script tags", () => {
@@ -391,7 +391,7 @@ describe("parseHtml", () => {
<html>
<body>
<div id="stage">
<div id="text1" data-start="0" data-end="5" data-keyframes='${keyframes}'><div>Hello</div></div>
<div id="text1" data-hf-id="text1" data-start="0" data-end="5" data-keyframes='${keyframes}'><div>Hello</div></div>
</div>
</body>
</html>
+13 -2
View File
@@ -11,6 +11,7 @@ import type {
CompositionVariable,
} from "../core.types";
import { validateCompositionGsap } from "./gsapSerialize";
import { ensureHfIds } from "./hfIds.js";
import type { ValidationResult } from "../core.types";
const MEDIA_TYPES = new Set<string>(["video", "image", "audio"]);
@@ -156,8 +157,9 @@ function resolveResolutionFromDimensions(width: number, height: number): CanvasR
}
export function parseHtml(html: string): ParsedHtml {
const withIds = ensureHfIds(html);
const parser = new DOMParser();
const doc = parser.parseFromString(html, "text/html");
const doc = parser.parseFromString(withIds, "text/html");
const elements: TimelineElement[] = [];
const keyframes: Record<string, Keyframe[]> = {};
@@ -190,7 +192,16 @@ export function parseHtml(html: string): ParsedHtml {
duration = 5;
}
const id = el.id || `element-${++idCounter}`;
// R1: stable hf- id minted by ensureHfIds above; clips just read it.
// Legacy/migration note: ensureHfIds pins a pre-existing `data-hf-id`, and
// the generator emits `data-hf-id="${element.id}"`. So a clip authored
// before R1 with `id="my-title"` round-trips as `data-hf-id="my-title"` —
// a non-`hf-`-shaped but still stable, exact-match handle. This is safe
// indefinitely: targeting uses exact `[data-hf-id="…"]` match (it does not
// require the hf- prefix). ensureHfIds skips elements that already carry
// data-hf-id, so legacy values are NOT re-minted automatically — they
// persist until the user re-saves the composition through Studio. Not a bug.
const id = el.getAttribute("data-hf-id") || el.id || `element-${++idCounter}`;
const name = getElementName(el);
const zIndex = getZIndex(el);
+6 -6
View File
@@ -18,7 +18,7 @@ import { serialize } from "./test-utils.js";
describe("T2 — stable element ids (spec for R1)", () => {
// --- Spec (red until R1) ---
it.fails("[spec] elements without an id get a hf- prefixed id at parse", () => {
it("[spec] elements without an id get a hf- prefixed id at parse", () => {
const html = `<html><body><div id="stage">
<img src="logo.svg" data-start="0" data-end="5" data-name="Logo" />
<div data-start="0" data-end="5" data-name="Card"><div>Text</div></div>
@@ -29,7 +29,7 @@ describe("T2 — stable element ids (spec for R1)", () => {
}
});
it.fails("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => {
it("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => {
const html = `<html><body><div id="stage">
<div data-start="0" data-end="5" data-name="Unnamed"><div>X</div></div>
<video data-start="1" data-end="6" src="v.mp4" data-name="Clip"></video>
@@ -41,7 +41,7 @@ describe("T2 — stable element ids (spec for R1)", () => {
}
});
it.fails("[spec] adding an element before existing ones does not change existing ids", () => {
it("[spec] adding an element before existing ones does not change existing ids", () => {
const base = `<html><body><div id="stage">
<div data-start="0" data-end="5" data-name="AlphaEl"><div>A</div></div>
<div data-start="1" data-end="6" data-name="BetaEl"><div>B</div></div>
@@ -62,12 +62,12 @@ describe("T2 — stable element ids (spec for R1)", () => {
// --- Baseline (already pass, must not regress) ---
it("elements with an existing id keep it unchanged", () => {
it("existing data-hf-id is pinned and becomes the clip id (never re-minted)", () => {
const html = `<html><body><div id="stage">
<div id="my-title" data-start="0" data-end="5" data-name="Title"><div>Hi</div></div>
<div data-hf-id="hf-anch" data-start="0" data-end="5" data-name="Title"><div>Hi</div></div>
</div></body></html>`;
const { elements } = parseHtml(html);
expect(elements.some((e) => e.id === "my-title")).toBe(true);
expect(elements.some((e) => e.id === "hf-anch")).toBe(true);
});
it("ids are deterministic: same input produces same ids on re-parse", () => {