Skip to content

Commit 42c247d

Browse files
vanceingallsclaude
andauthored
feat(studio): sourcePatcher data-hf-id targeting (R1, T3) (#1271)
* 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> * 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) <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8d6b7cf commit 42c247d

2 files changed

Lines changed: 127 additions & 49 deletions

File tree

packages/studio/src/utils/sourcePatcher.test.ts

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -517,17 +517,88 @@ describe("motion attribute round-trip via sourcePatcher", () => {
517517
});
518518
});
519519

520-
// T3 — id-based targeting (spec for R1).
521-
// R1 adds `hfId?: string` to PatchTarget and a `[data-hf-id="…"]` lookup branch
522-
// in findTagByTarget. Convert from it.todo to real assertions in the R1 PR.
520+
// T3 — id-based targeting (R1).
523521
describe("T3 — hfId targeting (spec for R1)", () => {
524-
it.todo("updates inline style by data-hf-id");
522+
it("updates inline style by data-hf-id", () => {
523+
const html = `<h1 data-hf-id="hf-x7k2" style="color: red">Hello</h1>`;
524+
const result = applyPatchByTarget(
525+
html,
526+
{ hfId: "hf-x7k2" },
527+
{
528+
type: "inline-style",
529+
property: "color",
530+
value: "blue",
531+
},
532+
);
533+
expect(result).toContain("color: blue");
534+
expect(result).toContain('data-hf-id="hf-x7k2"');
535+
});
525536

526-
it.todo("updates text content by data-hf-id");
537+
it("updates text content by data-hf-id", () => {
538+
const html = `<p data-hf-id="hf-a1b2">Old text</p>`;
539+
const result = applyPatchByTarget(
540+
html,
541+
{ hfId: "hf-a1b2" },
542+
{
543+
type: "text-content",
544+
property: "",
545+
value: "New text",
546+
},
547+
);
548+
expect(result).toContain(">New text<");
549+
});
527550

528-
it.todo("updates attribute by data-hf-id");
551+
it("updates attribute by data-hf-id", () => {
552+
const html = `<div data-hf-id="hf-c3d4" data-start="0"></div>`;
553+
const result = applyPatchByTarget(
554+
html,
555+
{ hfId: "hf-c3d4" },
556+
{
557+
type: "attribute",
558+
property: "start",
559+
value: "2.5",
560+
},
561+
);
562+
expect(result).toContain('data-start="2.5"');
563+
});
564+
565+
it("data-hf-id attribute is preserved after a style patch", () => {
566+
const html = `<h1 data-hf-id="hf-x7k2" style="color: red">Hello</h1>`;
567+
const patched = applyPatchByTarget(
568+
html,
569+
{ hfId: "hf-x7k2" },
570+
{
571+
type: "inline-style",
572+
property: "color",
573+
value: "blue",
574+
},
575+
);
576+
expect(readAttributeByTarget(patched, { hfId: "hf-x7k2" }, "data-hf-id")).toBe("hf-x7k2");
577+
});
529578

530-
it.todo("data-hf-id attribute is preserved after a style patch");
579+
it("hfId lookup falls through to selector when hfId not found", () => {
580+
const html = `<h1 class="headline" style="color: red">Hello</h1>`;
581+
const result = applyPatchByTarget(
582+
html,
583+
{ hfId: "hf-missing", selector: ".headline" },
584+
{ type: "inline-style", property: "color", value: "blue" },
585+
);
586+
expect(result).toContain("color: blue");
587+
});
531588

532-
it.todo("hfId lookup falls through to selector when hfId not found");
589+
it("hfId match is authoritative — selector is not used as a narrowing filter", () => {
590+
// hfId matches h1; selector points at h2. hfId wins — patch lands on h1, h2 untouched.
591+
const html = `<h1 data-hf-id="hf-x7k2" class="a">A</h1><h2 class="b">B</h2>`;
592+
const result = applyPatchByTarget(
593+
html,
594+
{ hfId: "hf-x7k2", selector: ".b" },
595+
{ type: "inline-style", property: "color", value: "blue" },
596+
);
597+
expect(result).toContain('data-hf-id="hf-x7k2"');
598+
const h1End = result.indexOf("</h1>");
599+
const bluePos = result.indexOf("color: blue");
600+
expect(bluePos).toBeGreaterThan(-1);
601+
expect(bluePos).toBeLessThan(h1End);
602+
expect(result).toContain('<h2 class="b">B</h2>');
603+
});
533604
});

packages/studio/src/utils/sourcePatcher.ts

Lines changed: 48 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export interface PatchOperation {
9494

9595
export interface PatchTarget {
9696
id?: string | null;
97+
hfId?: string;
9798
selector?: string;
9899
selectorIndex?: number;
99100
}
@@ -232,61 +233,67 @@ function replaceTagAtMatch(html: string, match: TagMatch, newTag: string): strin
232233
return `${html.slice(0, match.start)}${newTag}${html.slice(match.end)}`;
233234
}
234235

235-
export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null {
236-
if (target.id) {
237-
const idPattern = new RegExp(`(<[^>]*\\bid=(["'])${escapeRegex(target.id)}\\2[^>]*)>`, "i");
238-
const match = idPattern.exec(html);
239-
if (match?.index != null) {
236+
function execDataAttrPattern(html: string, attr: string, value: string): TagMatch | null {
237+
const pattern = new RegExp(`(<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\2[^>]*)>`, "i");
238+
const match = pattern.exec(html);
239+
if (match?.index == null) return null;
240+
// Defensive: a second exact match means a duplicate id/attr in the source
241+
// (id drift). Don't silently patch the first while leaving the other stale —
242+
// surface it. By the mint contract this should never fire.
243+
const all = html.match(new RegExp(`<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\1[^>]*>`, "gi"));
244+
if (all && all.length > 1) {
245+
// eslint-disable-next-line no-console
246+
console.warn(
247+
`sourcePatcher: ${attr}="${value}" matched ${all.length} elements; patching the first. ids/attrs must be unique per document.`,
248+
);
249+
}
250+
return { tag: match[1], start: match.index, end: match.index + match[1].length };
251+
}
252+
253+
function findTagByClass(html: string, target: PatchTarget): TagMatch | null {
254+
const classMatch = target.selector?.match(/^\.([a-zA-Z0-9_-]+)$/);
255+
if (!classMatch) return null;
256+
const cls = classMatch[1];
257+
const pattern = new RegExp(
258+
`(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`,
259+
"gi",
260+
);
261+
const selectorIndex = target.selectorIndex ?? 0;
262+
let match: RegExpExecArray | null;
263+
let currentIndex = 0;
264+
while ((match = pattern.exec(html)) !== null) {
265+
if (currentIndex === selectorIndex && match.index != null) {
240266
return {
241267
tag: match[1],
242268
start: match.index,
243269
end: match.index + match[1].length,
244270
};
245271
}
272+
currentIndex += 1;
273+
}
274+
return null;
275+
}
276+
277+
export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null {
278+
if (target.hfId) {
279+
const result = execDataAttrPattern(html, "data-hf-id", target.hfId);
280+
if (result) return result;
281+
}
282+
283+
if (target.id) {
284+
const result = execDataAttrPattern(html, "id", target.id);
285+
if (result) return result;
246286
}
247287

248288
if (!target.selector) return null;
249289

250290
const compositionIdMatch = target.selector.match(/^\[data-composition-id="([^"]+)"\]$/);
251291
if (compositionIdMatch) {
252-
const compId = compositionIdMatch[1];
253-
const pattern = new RegExp(
254-
`(<[^>]*\\bdata-composition-id=(["'])${escapeRegex(compId)}\\2[^>]*)>`,
255-
"i",
256-
);
257-
const match = pattern.exec(html);
258-
if (match?.index != null) {
259-
return {
260-
tag: match[1],
261-
start: match.index,
262-
end: match.index + match[1].length,
263-
};
264-
}
292+
const result = execDataAttrPattern(html, "data-composition-id", compositionIdMatch[1]);
293+
if (result) return result;
265294
}
266295

267-
const classMatch = target.selector.match(/^\.([a-zA-Z0-9_-]+)$/);
268-
if (classMatch) {
269-
const cls = classMatch[1];
270-
const pattern = new RegExp(
271-
`(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`,
272-
"gi",
273-
);
274-
const selectorIndex = target.selectorIndex ?? 0;
275-
let match: RegExpExecArray | null;
276-
let currentIndex = 0;
277-
while ((match = pattern.exec(html)) !== null) {
278-
if (currentIndex === selectorIndex && match.index != null) {
279-
return {
280-
tag: match[1],
281-
start: match.index,
282-
end: match.index + match[1].length,
283-
};
284-
}
285-
currentIndex += 1;
286-
}
287-
}
288-
289-
return null;
296+
return findTagByClass(html, target);
290297
}
291298

292299
export function readAttributeByTarget(

0 commit comments

Comments
 (0)