Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions packages/core/src/parsers/hfIds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,35 +100,34 @@ describe("ensureHfIds", () => {
});

// Lock the edit-lifecycle behavior. These pin BOTH the guarantee that holds
// once ids are persisted to source (pinning) AND the two limitations that hold
// while they are not (design §3 write-back is not yet wired — see
// notes/r1-stable-hf-ids-design.md "Implementation status & verified lifecycle gap").
// once ids are persisted to source (pinning) AND the behavior for truly unpinned
// HTML (no data-hf-id in the input — unreachable in production after write-back
// landed in R7 Task 1-2, but still the correct contract for that path).
describe("ensureHfIds — edit lifecycle (R1 stability)", () => {
it("pinned id survives a content edit (the §3 write-back guarantee)", () => {
// Element already carries data-hf-id in source (as it would after write-back).
const edited = doc(`<p class="body" data-hf-id="hf-abcd">Hello world</p>`);
expect(idOf(ensureHfIds(edited), "p.body")).toBe("hf-abcd");
});

it("KNOWN LIMITATION: an unpinned id changes when the element's text is edited", () => {
// No data-hf-id in source → every parse re-mints from content. Editing the
// text changes the hash, so the id drifts. This is the "pure-hash" mode the
// design rejected; flip this assertion to .toBe once write-back lands.
it("unpinned id drifts when element text is edited (pure-hash, unreachable after write-back)", () => {
// No data-hf-id in source → every parse re-mints from content. This path is
// unreachable in production after R7 write-back: the first serve pins the id.
const before = idOf(ensureHfIds(doc(`<p class="body">Hello</p>`)), "p.body");
const after = idOf(ensureHfIds(doc(`<p class="body">Hello world</p>`)), "p.body");
expect(before).not.toBe(after);
});

it("KNOWN LIMITATION: an unpinned id changes when an attribute is edited", () => {
it("unpinned id drifts when attribute is edited (pure-hash, unreachable after write-back)", () => {
const before = idOf(ensureHfIds(doc(`<p class="body">x</p>`)), "p");
const after = idOf(ensureHfIds(doc(`<p class="lead">x</p>`)), "p");
expect(before).not.toBe(after);
});

it("KNOWN LIMITATION: identical-content siblings have no content-stable id for the 2nd occurrence", () => {
it("identical-content siblings: second occurrence gets a position-derived dedup id", () => {
// Insertion stability holds for DISTINCT content (covered elsewhere), but a
// second identical sibling collides and gets a position-derived dedup id
// there is no content-stable handle for it. The first keeps the base id.
// second identical sibling collides and gets a position-derived dedup id.
// First element keeps the base (content-derived) id; documented in project_hfid_dedup_tiebreak.
const single = idOf(ensureHfIds(doc(`<p class="x">same</p>`)), "p.x");
const pair = ids(ensureHfIds(doc(`<p class="x">same</p><p class="x">same</p>`)));
expect(pair[0]).toBe(single); // first identical element: stable, content-derived
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/studio-api/helpers/sourceMutation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
removeElementFromHtml,
patchElementInHtml,
probeElementInSource,
splitElementInHtml,
} from "./sourceMutation.js";

describe("removeElementFromHtml", () => {
Expand Down Expand Up @@ -455,3 +456,14 @@ describe("T7 — data-hf-id targeting (spec for R1)", () => {
expect(html).toContain('data-hf-id="hf-a1b2"');
});
});

describe("splitElementInHtml — hfId clone isolation", () => {
it("does not copy data-hf-id to the cloned second half", () => {
const source = `<html><body><div data-composition-id="root"><div id="clip1" class="clip" data-start="0" data-duration="10" data-hf-id="hf-abc123"></div></div></body></html>`;
const { html, matched } = splitElementInHtml(source, { id: "clip1" }, 5, "clip2");

expect(matched).toBe(true);
const occurrences = (html.match(/data-hf-id="hf-abc123"/g) ?? []).length;
expect(occurrences).toBe(1);
});
});
1 change: 1 addition & 0 deletions packages/core/src/studio-api/helpers/sourceMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ export function splitElementInHtml(

const clone = el.cloneNode(true) as HTMLElement;
clone.setAttribute("id", newId);
clone.removeAttribute("data-hf-id");
clone.setAttribute("data-start", String(Math.round(splitTime * 1000) / 1000));
clone.setAttribute("data-duration", String(Math.round(secondDuration * 1000) / 1000));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @vitest-environment jsdom
import { describe, expect, it } from "vitest";
import { selectionCacheKey } from "./domEditOverlayGeometry";

describe("selectionCacheKey — hfId collision (R7)", () => {
it("produces distinct keys for two elements that differ only by hfId", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const a = selectionCacheKey({ sourceFile: "index.html", hfId: "hf-111" } as any);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const b = selectionCacheKey({ sourceFile: "index.html", hfId: "hf-222" } as any);
expect(a).not.toBe(b);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,11 @@ export function filterNestedDomEditGroupItems<T extends { element: HTMLElement }
}

export function selectionCacheKey(
selection: Pick<DomEditSelection, "id" | "selector" | "selectorIndex" | "sourceFile">,
selection: Pick<DomEditSelection, "id" | "hfId" | "selector" | "selectorIndex" | "sourceFile">,
): string {
return [
selection.sourceFile ?? "",
selection.hfId ?? "",
selection.id ?? "",
selection.selector ?? "",
selection.selectorIndex ?? "",
Expand Down
43 changes: 43 additions & 0 deletions packages/studio/src/components/editor/domEditing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1156,3 +1156,46 @@ describe("patch builders and prompt builder", () => {
).not.toThrow();
});
});

describe("hfId — find, key, capabilities (R7 fixes)", () => {
it("getDomEditTargetKey keeps two hfId-only elements distinct", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const a = getDomEditTargetKey({ sourceFile: "index.html", hfId: "hf-aaa" } as any);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const b = getDomEditTargetKey({ sourceFile: "index.html", hfId: "hf-bbb" } as any);
expect(a).not.toBe(b);
});

it("findElementForSelection finds element by data-hf-id when no id or selector", () => {
const doc = createDocument(`
<div data-composition-id="root">
<div data-hf-id="hf-xyz789" class="clip" style="position:absolute;left:0;top:0;width:100px;height:100px;"></div>
</div>
`);
const el = doc.querySelector('[data-hf-id="hf-xyz789"]') as HTMLElement;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const found = findElementForSelection(doc, { hfId: "hf-xyz789" } as any);
expect(found).toBe(el);
});

it("resolveDomEditCapabilities enables editing for hfId-only element (no CSS selector)", () => {
const result = resolveDomEditCapabilities({
// eslint-disable-next-line @typescript-eslint/no-explicit-any
hfId: "hf-abc" as any,
selector: undefined,
inlineStyles: { left: "10px", top: "20px", width: "100px", height: "50px" },
computedStyles: {
position: "absolute",
left: "10px",
top: "20px",
width: "100px",
height: "50px",
},
isCompositionHost: false,
isInsideLockedComposition: false,
isMasterView: false,
});
expect(result.canSelect).toBe(true);
expect(result.canMove).toBe(true);
});
});
1 change: 1 addition & 0 deletions packages/studio/src/components/editor/domEditing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export {
getDomEditNonEditableReason,
getDomEditTargetKey,
isTextEditableSelection,
readHfId,
refreshDomEditSelection,
resolveDomEditCapabilities,
resolveDomEditSelection,
Expand Down
10 changes: 8 additions & 2 deletions packages/studio/src/components/editor/domEditingElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function isInspectableLayerElement(el: HTMLElement): boolean {
export function getDomLayerPatchTarget(
el: HTMLElement,
activeCompositionPath: string | null,
): Pick<DomEditSelection, "id" | "selector" | "selectorIndex" | "sourceFile"> | null {
): Pick<DomEditSelection, "id" | "hfId" | "selector" | "selectorIndex" | "sourceFile"> | null {
if (!isInspectableLayerElement(el)) return null;
if (el.hasAttribute("data-composition-id")) return null;

Expand All @@ -105,6 +105,7 @@ export function getDomLayerPatchTarget(
const { sourceFile } = getSourceFileForElement(el, activeCompositionPath);
return {
id: el.id || undefined,
hfId: el.getAttribute("data-hf-id") || undefined,
selector,
selectorIndex: getSelectorIndex(
el.ownerDocument,
Expand Down Expand Up @@ -229,9 +230,14 @@ export function isLargeRasterDomEditSelection(

export function findElementForSelection(
doc: Document,
selection: Pick<DomEditSelection, "id" | "selector" | "selectorIndex" | "sourceFile">,
selection: Pick<DomEditSelection, "id" | "hfId" | "selector" | "selectorIndex" | "sourceFile">,
activeCompositionPath: string | null = null,
): HTMLElement | null {
if (selection.hfId) {
const byHfId = doc.querySelector(`[data-hf-id="${selection.hfId}"]`);
if (isHtmlElement(byHfId)) return byHfId;
}

if (selection.id) {
const byId = doc.getElementById(selection.id);
if (
Expand Down
27 changes: 26 additions & 1 deletion packages/studio/src/components/editor/domEditingLayers.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @vitest-environment jsdom
import { describe, expect, it } from "vitest";
import { resolveDomEditSelection, buildDomEditPatchTarget } from "./domEditingLayers";
import { resolveDomEditSelection, buildDomEditPatchTarget, readHfId } from "./domEditingLayers";

const opts = { activeCompositionPath: "index.html", isMasterView: true, skipSourceProbe: true };

Expand All @@ -27,6 +27,31 @@ describe("buildDomEditPatchTarget", () => {
});
});

describe("readHfId", () => {
it("returns the attribute value when present", () => {
const el = document.createElement("div");
el.setAttribute("data-hf-id", "hf-abc");
expect(readHfId(el)).toBe("hf-abc");
});

it("returns undefined when attribute is absent", () => {
const el = document.createElement("div");
expect(readHfId(el)).toBeUndefined();
});

it("returns undefined when attribute is empty string", () => {
const el = document.createElement("div");
el.setAttribute("data-hf-id", "");
expect(readHfId(el)).toBeUndefined();
});

it("returns undefined when attribute is whitespace-only", () => {
const el = document.createElement("div");
el.setAttribute("data-hf-id", " ");
expect(readHfId(el)).toBeUndefined();
});
});

describe("resolveDomEditSelection — hfId from data-hf-id", () => {
it("populates hfId from the element data-hf-id attribute", async () => {
const el = document.createElement("div");
Expand Down
36 changes: 22 additions & 14 deletions packages/studio/src/components/editor/domEditingLayers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export function buildDefaultDomEditTextField(base?: Partial<DomEditTextField>):
// fallow-ignore-next-line complexity
export function resolveDomEditCapabilities(args: {
selector?: string;
hfId?: string;
tagName?: string;
className?: string;
inlineStyles: Record<string, string>;
Expand All @@ -182,7 +183,7 @@ export function resolveDomEditCapabilities(args: {
isMasterView: boolean;
existsInSource?: boolean;
}): DomEditCapabilities {
if (!args.selector || args.isInsideLockedComposition) {
if ((!args.selector && !args.hfId) || args.isInsideLockedComposition) {
return {
canSelect: !args.isInsideLockedComposition,
canEditStyles: false,
Expand Down Expand Up @@ -289,7 +290,7 @@ export function buildElementLabel(el: HTMLElement): string {
async function probeSourceElement(
projectId: string,
sourceFile: string,
target: { id?: string; selector?: string; selectorIndex?: number },
target: { id?: string; hfId?: string; selector?: string; selectorIndex?: number },
): Promise<boolean> {
try {
const response = await fetch(
Expand Down Expand Up @@ -321,7 +322,8 @@ export async function resolveDomEditSelection(
let current: HTMLElement | null = getSelectionCandidate(startEl, options);
while (current && current !== doc.body && current !== doc.documentElement) {
const selector = buildStableSelector(current);
if (!selector) {
const hfId = readHfId(current);
if (!selector && !hfId) {
current = current.parentElement;
continue;
}
Expand All @@ -330,13 +332,9 @@ export async function resolveDomEditSelection(
current,
options.activeCompositionPath,
);
const selectorIndex = getSelectorIndex(
doc,
current,
selector,
sourceFile,
options.activeCompositionPath,
);
const selectorIndex = selector
? getSelectorIndex(doc, current, selector, sourceFile, options.activeCompositionPath)
: undefined;
const compositionSrc =
current.getAttribute("data-composition-src") ??
current.getAttribute("data-composition-file") ??
Expand All @@ -346,15 +344,18 @@ export async function resolveDomEditSelection(
const textFields = collectDomEditTextFields(current);
const isInsideLocked = Boolean(findClosestByAttribute(current, ["data-timeline-locked"]));
let existsInSource: boolean | undefined;
if (!options.skipSourceProbe && options.projectId && (current.id || selector)) {
const probeTarget: { id?: string; selector?: string; selectorIndex?: number } = {};
if (!options.skipSourceProbe && options.projectId && (current.id || selector || hfId)) {
const probeTarget: { id?: string; hfId?: string; selector?: string; selectorIndex?: number } =
{};
if (current.id) probeTarget.id = current.id;
if (hfId) probeTarget.hfId = hfId;
if (selector) probeTarget.selector = selector;
if (selectorIndex != null) probeTarget.selectorIndex = selectorIndex;
existsInSource = await probeSourceElement(options.projectId, sourceFile, probeTarget);
}
const capabilities = resolveDomEditCapabilities({
selector,
hfId,
tagName: current.tagName.toLowerCase(),
className: current.className,
inlineStyles,
Expand All @@ -369,7 +370,7 @@ export async function resolveDomEditSelection(
return {
element: current,
id: current.id || undefined,
hfId: current.getAttribute("data-hf-id") ?? undefined,
hfId,
selector,
selectorIndex,
sourceFile,
Expand Down Expand Up @@ -452,6 +453,7 @@ export function collectDomEditLayerItems(
if (!root) return [];

const items: DomEditLayerItem[] = [];
// fallow-ignore-next-line complexity
const visit = (el: HTMLElement, depth: number) => {
if (items.length >= maxItems) return;

Expand All @@ -465,6 +467,7 @@ export function collectDomEditLayerItems(
depth,
childCount: getDirectLayerChildren(el, options).length,
id: target.id ?? undefined,
hfId: target.hfId ?? undefined,
selector: target.selector ?? undefined,
selectorIndex: target.selectorIndex,
sourceFile: target.sourceFile,
Expand Down Expand Up @@ -536,10 +539,11 @@ export function getDomEditNonEditableReason(
}

export function getDomEditTargetKey(
selection: Pick<DomEditSelection, "id" | "selector" | "selectorIndex" | "sourceFile">,
selection: Pick<DomEditSelection, "id" | "hfId" | "selector" | "selectorIndex" | "sourceFile">,
): string {
return [
selection.sourceFile || "index.html",
selection.hfId ?? "",
selection.id ?? "",
selection.selector ?? "",
selection.selectorIndex ?? "",
Expand All @@ -556,6 +560,10 @@ export function isTextEditableSelection(selection: DomEditSelection): boolean {

// buildElementAgentPrompt is in domEditingAgentPrompt.ts

export function readHfId(element: Element): string | undefined {
return element.getAttribute("data-hf-id")?.trim() || undefined;
}

export function buildDomEditPatchTarget(
selection: Pick<DomEditSelection, "id" | "hfId" | "selector" | "selectorIndex">,
): { id?: string | null; hfId?: string; selector?: string; selectorIndex?: number } {
Expand Down
1 change: 1 addition & 0 deletions packages/studio/src/components/editor/domEditingTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export interface DomEditLayerItem {
depth: number;
childCount: number;
id?: string;
hfId?: string;
selector?: string;
selectorIndex?: number;
sourceFile: string;
Expand Down
5 changes: 3 additions & 2 deletions packages/studio/src/hooks/useDomEditCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { primaryFontFamilyValue } from "../utils/studioFontHelpers";
import {
buildDomEditPatchTarget,
getDomEditTargetKey,
readHfId,
type DomEditSelection,
} from "../components/editor/domEditing";
import {
Expand Down Expand Up @@ -533,7 +534,7 @@ export function useDomEditCommits({
}>,
) => {
if (entries.length === 0) return;
const coalesceKey = `z-reorder:${entries.map((e) => e.id ?? e.selector ?? "el").join(":")}`;
const coalesceKey = `z-reorder:${entries.map((e) => e.id ?? e.selector ?? e.element.getAttribute("data-hf-id") ?? "el").join(":")}`;
for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
entry.element.style.zIndex = String(entry.zIndex);
Expand All @@ -553,7 +554,7 @@ export function useDomEditCommits({
{
element: entry.element,
id: entry.id ?? null,
hfId: entry.element.getAttribute("data-hf-id") ?? undefined,
hfId: readHfId(entry.element),
selector: entry.selector,
selectorIndex: entry.selectorIndex,
sourceFile: entry.sourceFile,
Expand Down
1 change: 1 addition & 0 deletions packages/studio/src/hooks/useGsapScriptCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ export function useGsapScriptCommits({
body: JSON.stringify({
target: {
id: selection.id,
hfId: selection.hfId,
selector: selection.selector,
selectorIndex: selection.selectorIndex,
},
Expand Down
Loading
Loading