feat(studio,core): populate hfId in DomEditSelection + widen MutationTarget (R7, T5a)

[vanceingalls]

Summary

• Adds hfId field to resolveDomEditSelection — reads data-hf-id off the live element and stores it in DomEditSelection.hfId
• DomEditSelection extends PatchTarget which already declares hfId?: string, so this is a single new line at the return site
• Widens MutationTarget in files.ts to include hfId?: string (type hygiene — the value already survives through parseMutationBody's by-reference pass, so this is documentation not a behaviour change)

Why

R7 / Task 5a. The full hf-id write-back and patch-engine infrastructure (R1 + R7 Tasks 0–4, PRs #1269–#1292) is server-complete. The only missing piece was: the Studio client never read data-hf-id off a hit-tested element, so target.hfId was always undefined and the hfId-first lookup branches in both patch engines were unreachable in production. This PR fixes the selection side — the commit wire (#1297) completes the path.

Test plan

• packages/studio/src/components/editor/domEditingLayers.test.ts — two new tests with jsdom environment:
  • resolveDomEditSelection on an element with data-hf-id → selection.hfId is populated
  • element without data-hf-id → selection.hfId is undefined
• All 65 studio test files pass, all 72 core test files pass

🤖 Generated with Claude Code

[vanceingalls]

• feat(studio): carry hfId on TimelineElement, wire through buildPatchTarget (R7, T5b) #1299
• feat(studio): wire hfId through DOM-edit patch targets, activate hfId lookup path (R7, T5a) #1297
• feat(studio,core): populate hfId in DomEditSelection + widen MutationTarget (R7, T5a) #1296 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

Clean and minimal. getAttribute("data-hf-id") ?? undefined is the right call — getAttribute returns null not undefined on miss, so the coalesce is necessary. The MutationTarget widen in files.ts is pure type hygiene as described. Tests cover both paths.

✅ Approve.

[james-russo-rames-d-jusso]

Reviewed at 8996493. Stack 1/3 — foundational widening (+37/-1, 3 files). Tiny diff, correct shape. The DomEditSelection extends PatchTarget relationship was already in place (domEditingTypes.ts:74 + sourcePatcher.ts:95 declares hfId?: string), and findByHfId on the server side has been the primary lookup since R1 (sourceMutation.ts:43-66) — this PR is the missing one-line read on the studio side that makes target.hfId actually arrive populated. The MutationTarget widening in files.ts is pure type hygiene (the unvalidated parseMutationBody<T> cast at L112-120 means any field already survives by-reference), so the only behavior change is the studio-side getAttribute read.

End-to-end check on Home's review angles:

• Server acceptance shape (angle #2): ✓ R1's findByHfId already keys off target.hfId first, fallback to id/selector exists, CSS-escape is in place. Server is ready.
• Type-shape parity (angle #3): ✓ PatchTarget.hfId?: string (FE) and MutationTarget.hfId?: string (server) both optional. Match.
• Cutover ordering (angle #4): ✓ Safe in any order. If this lands before #1297, selection.hfId is populated but no commit sends it — no behavior change. If #1297 lands before R1 (hypothetically), server fallback to id/selector matches existing behavior. R1 is already merged so this is theoretical, but the path is clean.
• Backwards-compat (angle #6): ✓ Legacy elements without data-hf-id produce selection.hfId === undefined, both engines coexist.
• Test coverage (angle #7): Two tests for the happy path and missing-attribute path. Reasonable for the size of the change; would extend with one more case below.

No blockers. Three minor things:

Concerns

1. Empty-string data-hf-id becomes selection.hfId === "", not undefined

domEditingLayers.ts:372:

hfId: current.getAttribute("data-hf-id") ?? undefined,

Element.getAttribute() returns null when the attribute is missing but the literal string "" when the attribute is present-but-empty (<div data-hf-id="">). The ?? undefined only converts null → undefined; an empty string passes through as "".

Downstream, findByHfId (sourceMutation.ts:43) would then build the selector [data-hf-id=""], which matches all elements with an explicitly-empty data-hf-id. The first match wins, with a multi-match console warn. This could silently target a different element than the user selected.

Concrete trigger: a malformed serializer output, a clip authored by hand with data-hf-id="", or a bug that strips id values without removing the attribute. Unlikely in normal flow, but not impossible.

Fix (one-liner):

hfId: current.getAttribute("data-hf-id")?.trim() || undefined,

The || undefined (instead of ?? undefined) catches both null and "". The optional .trim() also handles data-hf-id=" " (whitespace-only).

2. Test coverage misses the empty-string case

domEditingLayers.test.ts:7-22 covers:

• Element with data-hf-id="hf-x7k2" → hfId === "hf-x7k2" ✓
• Element without data-hf-id → hfId === undefined ✓

Missing:

• data-hf-id="" (present, empty) — currently produces "", would produce undefined after the fix above.
• data-hf-id=" " (whitespace) — same.

Adding 5-10 lines for either case would pin the contract.

3. parseMutationBody's unvalidated cast surfaces a soft trust boundary

files.ts:112-120 reads the JSON body and casts to T with no runtime validation. This is what makes the MutationTarget widening a no-op at runtime — the body shape is whatever the client sends. For hfId specifically, the CSS-escape in sourceMutation.findByHfId (sourceMutation.ts:32-41 per my #1272 review) is the actual safety boundary.

Not a blocker — the wiring is correct — but worth a one-line comment at the MutationTarget definition saying "runtime validation lives in sourceMutation.findByHfId (CSS attr-value escape) — this type is documentation only." Helps the next reader understand why widening the type doesn't require a corresponding validation update.

What I didn't verify

• The full chain into #1297 / #1299 where target.hfId is actually sent on the wire — those are the next PRs in this stack, so the integration test is implicit in those review angles.
• The R1 PR (#1271/#1272) deployment status — assumed merged + rolled out, since this PR's body says "server-complete."
• Multi-source-file flow (the legacy elements migration path) — but the briefing's angle #6 (backwards-compat) is handled correctly by the optional-undefined semantics.

Clean foundational change. The empty-string handling is the only real wart; trivial to fix.

— Review by Rames D Jusso