refactor(studio): extract readHfId helper, fix empty-string normalization, add comments (R7 review)

- Extract readHfId(el) to domEditingLayers.ts — centralizes `?.trim() || undefined`
  normalisation; guards against empty-string data-hf-id reaching findTagByTarget
- Wire readHfId into domEditingLayers.ts and useDomEditCommits.ts (the one site
  that still used `?? undefined` instead of `|| undefined`)
- Re-export readHfId through domEditing.ts public API
- Add readHfId unit tests: present, absent, empty-string, whitespace-only
- Add comment on PatchTarget: runtime validation lives in findTagByTarget, type is docs-only
- Suppress pre-existing unused re-exports in timelineDOM.ts (backward-compat re-exports
  brought into fallow scope by the T5b hfId changes)

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

Author: vanceingalls

packages/studio/src/components/editor/domEditing.ts

@@ -35,6 +35,7 @@ export {
   getDomEditNonEditableReason,
   getDomEditTargetKey,
   isTextEditableSelection,
+  readHfId,
   refreshDomEditSelection,
   resolveDomEditCapabilities,
   resolveDomEditSelection,

packages/studio/src/components/editor/domEditingLayers.test.ts

@@ -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 };
 
@@ -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");

packages/studio/src/components/editor/domEditingLayers.ts

@@ -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>;
@@ -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,
@@ -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(
@@ -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;
     }
@@ -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") ??
@@ -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,
@@ -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,
@@ -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;
 
@@ -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,
@@ -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 ?? "",
@@ -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 } {

packages/studio/src/hooks/useDomEditCommits.ts

@@ -8,6 +8,7 @@ import { primaryFontFamilyValue } from "../utils/studioFontHelpers";
 import {
   buildDomEditPatchTarget,
   getDomEditTargetKey,
+  readHfId,
   type DomEditSelection,
 } from "../components/editor/domEditing";
 import {
@@ -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);
@@ -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,

packages/studio/src/player/lib/timelineDOM.ts

@@ -26,14 +26,21 @@ import {
 
 // Re-export helpers that were previously public from this module so that
 // existing import sites (hook + tests) don't need to change.
+// fallow-ignore-next-line unused-exports
 export {
   readTimelineDurationFromDocument,
+  // fallow-ignore-next-line unused-exports
   resolveMediaElement,
+  // fallow-ignore-next-line unused-exports
   applyMediaMetadataFromElement,
   getTimelineElementSelector,
+  // fallow-ignore-next-line unused-exports
   getTimelineElementSourceFile,
+  // fallow-ignore-next-line unused-exports
   getTimelineElementSelectorIndex,
+  // fallow-ignore-next-line unused-exports
   buildTimelineElementIdentity,
+  // fallow-ignore-next-line unused-exports
   getTimelineElementIdentity,
   findTimelineDomNodeForClip,
 } from "./timelineElementHelpers";
@@ -75,7 +82,7 @@ export function createTimelineElementFromManifestClip(params: {
   let hfId: string | undefined;
   if (hostEl) {
     domId = hostEl.id || undefined;
-    hfId = hostEl.getAttribute("data-hf-id") ?? undefined;
+    hfId = hostEl.getAttribute("data-hf-id") || undefined;
     selector = getTimelineElementSelector(hostEl);
     selectorIndex =
       doc && selector ? getTimelineElementSelectorIndex(doc, hostEl, selector) : undefined;
@@ -130,7 +137,7 @@ export function createTimelineElementFromManifestClip(params: {
     }
     if (hostEl) {
       entry.domId = hostEl.id || undefined;
-      entry.hfId = hostEl.getAttribute("data-hf-id") ?? undefined;
+      entry.hfId = hostEl.getAttribute("data-hf-id") || undefined;
       entry.selector = getTimelineElementSelector(hostEl);
       entry.selectorIndex =
         doc && entry.selector
@@ -191,7 +198,7 @@ export function createImplicitTimelineLayersFromDOM(
 
     layers.push({
       domId: child.id || undefined,
-      hfId: child.getAttribute("data-hf-id") ?? undefined,
+      hfId: child.getAttribute("data-hf-id") || undefined,
       duration: rootDuration,
       id: identity.id,
       key: identity.key,
@@ -267,7 +274,7 @@ export function parseTimelineFromDOM(doc: Document, rootDuration: number): Timel
       duration: dur,
       track: isNaN(track) ? 0 : track,
       domId: el.id || undefined,
-      hfId: el.getAttribute("data-hf-id") ?? undefined,
+      hfId: el.getAttribute("data-hf-id") || undefined,
       selector,
       selectorIndex,
       sourceFile,

packages/studio/src/utils/sourcePatcher.ts

@@ -92,6 +92,8 @@ export interface PatchOperation {
   value: string | null;
 }
 
+// Runtime validation for hfId lives in findTagByTarget → execDataAttrPattern (CSS attr-value
+// escape). This type is documentation only; the server's MutationTarget mirrors this shape.
 export interface PatchTarget {
   id?: string | null;
   hfId?: string;