From 740a83abb370cb7bc6d81d19f0a8b4dd37260717 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 9 Jun 2026 00:28:52 -0700 Subject: [PATCH] feat(core): hf-id write-back to disk + serve-time surfacing (R7, Tasks 1-2) (#1289) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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 * feat(studio): sourcePatcher data-hf-id targeting (R1, T3) * fix(studio): warn on duplicate match in execDataAttrPattern (R1, T3 review) Addresses Rames' review on #1271: execDataAttrPattern returned the first regex match without checking for a second. A duplicate id/data-hf-id in source (id drift) would silently patch one element and leave the other stale. Now warns when more than one element matches. By the mint contract it should never fire. Co-Authored-By: Claude Opus 4.8 (1M context) * test(studio): pin hfId-is-authoritative-over-selector contract (R1, T3 review) Adds test: "hfId match is authoritative — selector is not used as a narrowing filter". When hfId matches element A and selector points at element B, findTagByTarget returns A without consulting selector as a narrowing filter. Pins the intended behaviour so a future refactor cannot silently start narrowing by selector. Co-Authored-By: Claude Sonnet 4.6 * feat(core): sourceMutation data-hf-id targeting (R1, T7) * test(core): update htmlParser baselines for R1 hf- id format Elements now get data-hf-id minted by ensureHfIds; parser reads data-hf-id as model id, so HTML id attrs are no longer the model id. Co-Authored-By: Claude Sonnet 4.6 * test(core): data-hf-id survives id/selector patch (R1, T7) Locks the preservation guarantee the write-back design depends on: a Studio edit targeting by id or selector (it never sends hfId) must not strip an existing data-hf-id, or the stable handle is destroyed by the next edit. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(core): escape hfId in selector + warn on duplicate match (R1, T7 review) Addresses review on #1272 (Miguel P3 + Rames): findTargetElement interpolated target.hfId raw into a [data-hf-id="..."] selector. Escape it (CSS attr-value injection guard) and warn when a hfId matches more than one element instead of silently patching an arbitrary one. Adds an injection-guard test. Co-Authored-By: Claude Opus 4.8 (1M context) * test(core): previewAdapter contract failing tests (T10 spec for R7) * feat(core): hf-id write-back to disk + serve-time surfacing (R7, Task 1-2) * test(core): replace tautological stability tests with real disk tests for persistHfIdsIfNeeded Prior tests only exercised normalizeHfIds (pure function) and the existing pin guard in ensureHfIds — both pass on the parent commit without any Task 1 code. Replace with three tests that exercise the actual disk write-back: - writes data-hf-id to disk when source is untagged - does not rewrite disk when source is already tagged (idempotent) - returned id matches id written to disk (serve-time == persist-time invariant) These fail on the parent commit (persistHfIdsIfNeeded doesn't exist) and green after Task 1. Co-Authored-By: Claude Sonnet 4.6 * test(core): route-level tests for data-hf-id surfacing and disk write-back (R7, Task 1-2) Two integration tests against the preview route (via Hono test harness): - served HTML carries data-hf-id on body elements (>= 2 matches for div+p) - disk file contains data-hf-id after first GET (write-back verified via readFileSync) These fail on the parent commit (no hfIdPersist wiring in preview.ts) and green after Task 1. Closes the verification gap flagged in review. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Opus 4.8 (1M context) --- packages/core/src/parsers/hfIds.test.ts | 18 +++++ packages/core/src/parsers/hfIds.ts | 13 ++++ .../studio-api/helpers/hfIdPersist.test.ts | 69 +++++++++++++++++++ .../src/studio-api/helpers/hfIdPersist.ts | 21 ++++++ .../src/studio-api/routes/preview.test.ts | 33 +++++++++ .../core/src/studio-api/routes/preview.ts | 32 ++++++--- 6 files changed, 176 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/studio-api/helpers/hfIdPersist.test.ts create mode 100644 packages/core/src/studio-api/helpers/hfIdPersist.ts diff --git a/packages/core/src/parsers/hfIds.test.ts b/packages/core/src/parsers/hfIds.test.ts index f4125be8f..6001afe08 100644 --- a/packages/core/src/parsers/hfIds.test.ts +++ b/packages/core/src/parsers/hfIds.test.ts @@ -77,6 +77,24 @@ describe("ensureHfIds", () => { expect(a).toMatch(/^hf-[a-z0-9]{4}$/); expect(b).toMatch(/^hf-[a-z0-9]{4}$/); }); + + // Post-persist stability: once data-hf-id is written back to source, edits + // don't drift the id because the attribute is already present and pinned. + it("pinned id survives text edit after first persist", () => { + const raw = `
original text
`; + const persisted = ensureHfIds(raw); // simulates write-back on first serve + const [originalId] = ids(persisted); + const edited = persisted.replace("original text", "edited text"); + expect(ids(ensureHfIds(edited))).toContain(originalId); + }); + + it("pinned id survives attribute edit after first persist", () => { + const raw = `
text
`; + const persisted = ensureHfIds(raw); // simulates write-back on first serve + const [originalId] = ids(persisted); + const edited = persisted.replace('class="old"', 'class="new"'); + expect(ids(ensureHfIds(edited))).toContain(originalId); + }); }); // Lock the edit-lifecycle behavior. These pin BOTH the guarantee that holds diff --git a/packages/core/src/parsers/hfIds.ts b/packages/core/src/parsers/hfIds.ts index 6f005eeb7..7b1f9cff4 100644 --- a/packages/core/src/parsers/hfIds.ts +++ b/packages/core/src/parsers/hfIds.ts @@ -54,6 +54,19 @@ function contentKey(el: Element): string { return `${el.tagName.toLowerCase()}|${attrs}|${ownText(el)}`; } +/** + * Collision tiebreak for byte-identical siblings: document-order dup counter + * (`hash(key#N)`). This IS order-dependent — two identical `` + * get different ids based on which comes first in the DOM. This is unavoidable: + * unique ids for byte-identical elements require a positional signal. + * + * Why this is safe in practice: once `ensureHfIds` write-back persists + * `data-hf-id` to source the attribute is physically bound to its element. + * Reordering identical siblings carries the attribute along → zero + * order-dependence post-persist. `ensureHfIds` skips pinned elements + * (`if (el.getAttribute("data-hf-id")) continue`), so normal operation + * never re-exposes the ordering after first persist. + */ export function mintHfId(el: Element, assigned: Set): string { const key = contentKey(el); let id = toHfId(fnv1a(key)); diff --git a/packages/core/src/studio-api/helpers/hfIdPersist.test.ts b/packages/core/src/studio-api/helpers/hfIdPersist.test.ts new file mode 100644 index 000000000..e0e0ab154 --- /dev/null +++ b/packages/core/src/studio-api/helpers/hfIdPersist.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { normalizeHfIds, persistHfIdsIfNeeded } from "./hfIdPersist.js"; + +describe("normalizeHfIds", () => { + it("marks changed=true and adds data-hf-id to all body elements when untagged", () => { + const raw = `

hello

`; + const { html, changed } = normalizeHfIds(raw); + expect(changed).toBe(true); + expect(html).toContain('data-hf-id="hf-'); + const matches = html.match(/data-hf-id="hf-[a-z0-9]{4}"/g); + expect(matches?.length).toBeGreaterThanOrEqual(2); + }); + + it("marks changed=false for already-normalized HTML (idempotent round-trip)", () => { + const raw = `

hello

`; + const first = normalizeHfIds(raw).html; + const { html, changed } = normalizeHfIds(first); + expect(changed).toBe(false); + expect(html).toBe(first); + }); +}); + +describe("persistHfIdsIfNeeded", () => { + const tmpDirs: string[] = []; + + afterEach(() => { + for (const d of tmpDirs) rmSync(d, { recursive: true, force: true }); + tmpDirs.length = 0; + }); + + function tmpFile(content: string): string { + const dir = mkdtempSync(join(tmpdir(), "hfid-test-")); + tmpDirs.push(dir); + const file = join(dir, "index.html"); + writeFileSync(file, content, "utf-8"); + return file; + } + + it("writes data-hf-id to disk when source is untagged", () => { + const raw = `
hello
`; + const file = tmpFile(raw); + const returned = persistHfIdsIfNeeded(file, raw); + expect(returned).toContain('data-hf-id="hf-'); + const onDisk = readFileSync(file, "utf-8"); + expect(onDisk).toContain('data-hf-id="hf-'); + expect(onDisk).toBe(returned); + }); + + it("does not rewrite disk when source is already tagged", () => { + const raw = `
hello
`; + const file = tmpFile(raw); + const tagged = persistHfIdsIfNeeded(file, raw); + const diskAfterFirst = readFileSync(file, "utf-8"); + const returned2 = persistHfIdsIfNeeded(file, tagged); + expect(returned2).toBe(tagged); + expect(readFileSync(file, "utf-8")).toBe(diskAfterFirst); + }); + + it("returned id matches id written to disk (serve-time == persist-time invariant)", () => { + const raw = `text`; + const file = tmpFile(raw); + const result = persistHfIdsIfNeeded(file, raw); + const onDisk = readFileSync(file, "utf-8"); + expect(result).toBe(onDisk); + }); +}); diff --git a/packages/core/src/studio-api/helpers/hfIdPersist.ts b/packages/core/src/studio-api/helpers/hfIdPersist.ts new file mode 100644 index 000000000..8ec6bfdc4 --- /dev/null +++ b/packages/core/src/studio-api/helpers/hfIdPersist.ts @@ -0,0 +1,21 @@ +import { ensureHfIds } from "../../parsers/hfIds.js"; +import { writeFileSync } from "node:fs"; + +export { ensureHfIds }; + +export function normalizeHfIds(html: string): { html: string; changed: boolean } { + const normalized = ensureHfIds(html); + return { html: normalized, changed: normalized !== html }; +} + +export function persistHfIdsIfNeeded(filePath: string, html: string): string { + const { html: normalized, changed } = normalizeHfIds(html); + if (changed) { + try { + writeFileSync(filePath, normalized, "utf-8"); + } catch { + // non-fatal — serve with ids even if persist fails + } + } + return normalized; +} diff --git a/packages/core/src/studio-api/routes/preview.test.ts b/packages/core/src/studio-api/routes/preview.test.ts index 54cd0a6c4..c2c6ca5b2 100644 --- a/packages/core/src/studio-api/routes/preview.test.ts +++ b/packages/core/src/studio-api/routes/preview.test.ts @@ -328,3 +328,36 @@ describe("registerPreviewRoutes", () => { expect(signature).toMatch(/^[a-f0-9]{24}$/); }); }); + +describe("hf-id surfacing in preview route", () => { + it("serves HTML with data-hf-id on body elements (R7 write-back)", async () => { + const projectDir = createProjectDir(); + writeFileSync( + join(projectDir, "index.html"), + `

text

`, + ); + const app = new Hono(); + registerPreviewRoutes(app, createAdapter(projectDir)); + const res = await app.request("http://localhost/projects/demo/preview"); + expect(res.status).toBe(200); + const html = await res.text(); + const ids = html.match(/data-hf-id="hf-[a-z0-9]{4}"/g); + // div and p both tagged + expect(ids?.length).toBeGreaterThanOrEqual(2); + }); + + it("writes data-hf-id back to disk on first serve", async () => { + const { readFileSync } = await import("node:fs"); + const projectDir = createProjectDir(); + const indexPath = join(projectDir, "index.html"); + writeFileSync( + indexPath, + `
hello
`, + ); + const app = new Hono(); + registerPreviewRoutes(app, createAdapter(projectDir)); + await app.request("http://localhost/projects/demo/preview"); + const onDisk = readFileSync(indexPath, "utf-8"); + expect(onDisk).toContain('data-hf-id="hf-'); + }); +}); diff --git a/packages/core/src/studio-api/routes/preview.ts b/packages/core/src/studio-api/routes/preview.ts index 30da87ba6..ad74ec2f8 100644 --- a/packages/core/src/studio-api/routes/preview.ts +++ b/packages/core/src/studio-api/routes/preview.ts @@ -11,6 +11,7 @@ import { createStudioMotionRenderBodyScript, STUDIO_MOTION_PATH, } from "../helpers/studioMotionRenderScript.js"; +import { ensureHfIds, persistHfIdsIfNeeded } from "../helpers/hfIdPersist.js"; const PROJECT_SIGNATURE_META = "hyperframes-project-signature"; const GSAP_CDN_VERSION = "3.15.0"; @@ -205,14 +206,19 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi return new Response(null, { status: 304, headers: previewCacheHeaders(etag) }); } + // Normalize + persist data-hf-id to disk before bundle reads it. Idempotent. + const diskMain = resolveProjectMainHtml(project.dir, project.id); + const normalizedDisk = diskMain + ? persistHfIdsIfNeeded(join(project.dir, diskMain.compositionPath), diskMain.html) + : null; + try { let bundled = await adapter.bundle(project.dir); let mainCompositionPath = "index.html"; if (!bundled) { - const main = resolveProjectMainHtml(project.dir, project.id); - if (!main) return c.text("not found", 404); - bundled = main.html; - mainCompositionPath = main.compositionPath; + if (!diskMain || normalizedDisk === null) return c.text("not found", 404); + bundled = normalizedDisk; + mainCompositionPath = diskMain.compositionPath; } // Inject runtime if not already present (check URL pattern and bundler attribute) @@ -233,21 +239,27 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi } bundled = injectStudioPreviewAugmentations( - await transformPreviewHtml(bundled, adapter, project, mainCompositionPath), + ensureHfIds(await transformPreviewHtml(bundled, adapter, project, mainCompositionPath)), adapter, project.dir, mainCompositionPath, ); return c.html(bundled, 200, previewCacheHeaders(etag)); } catch { - const main = resolveProjectMainHtml(project.dir, project.id); - if (main) { + if (diskMain && normalizedDisk !== null) { return c.html( injectStudioPreviewAugmentations( - await transformPreviewHtml(main.html, adapter, project, main.compositionPath), + ensureHfIds( + await transformPreviewHtml( + normalizedDisk, + adapter, + project, + diskMain.compositionPath, + ), + ), adapter, project.dir, - main.compositionPath, + diskMain.compositionPath, ), 200, previewCacheHeaders(etag), @@ -284,7 +296,7 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi const baseHref = `/api/projects/${project.id}/preview/`; let html = buildSubCompositionHtml(project.dir, compPath, adapter.runtimeUrl, baseHref); if (!html) return c.text("not found", 404); - html = await transformPreviewHtml(html, adapter, project, compPath); + html = ensureHfIds(await transformPreviewHtml(html, adapter, project, compPath)); return c.html( injectStudioPreviewAugmentations(html, adapter, project.dir, compPath), 200,