feat(studio): link a video and its audio with data-link (6/8)

[vanceingalls]

Summary

Linked clips (data-link): a video and its audio can be linked so they select, drag, trim, split and delete together. Clips can be detached, linked, unlinked and merged back from the clip menu.

Changes

• core: @hyperframes/core/media-link helpers.
• lint: warnings for out-of-sync and orphaned linked clips.
• studio: link-aware selection, drag and trim. Source transforms for detach, merge, link and unlink. Link-aware split and delete, Alt-edit unlinks one member, and link edit actions go through one undo step. Clip menu gets Detach / Unlink / Link / Merge back / Delete this clip only.
• studio-server: split gives each right half of a linked pair its own lk-N link.
• sdk: setTiming applies to data-link partners by default.
• skills: hyperframes-core documents data-link.

Testing

Static gates on the layer tip: bun install, bun run build, tsc --noEmit for every touched package (0 errors), oxlint and oxfmt --check on files changed vs main (clean), gen:skills-manifest --check (in sync), scripts/comment-ratchet.mjs (ok).

• Scoped vitest: core 3804, lint 760, sdk 569, studio 6333, studio-server 998. All pass.

Notes

Part of the audio-on-video stack. See the bottom PR for the overview.

🤖 Generated with Claude Code

Before

After

Linked video menu: Unlink, Merge audio back into video, Delete this clip only; Inspector shows both elements.

[miguel-heygen]

Reviewed incremental #4818 at dddeeddfd677618b68b0aa2c5abf81ac19910c55 against 65b1bf6d3846389aa251407268c23e3fa65c36f7.

Detach copies rate automation onto both halves while moving sound-only settings off the video (packages/studio/src/components/editor/mediaLinkEdits.ts:116), and atomic split gives the right halves a fresh shared link (packages/studio-server/src/routes/files.ts:2211).

Blocker: packages/studio/src/components/editor/mediaLinkEdits.ts:190 — The new destructive merge uses mediaFileKey, which compares only a lowercased basename (packages/studio/src/player/components/audioClipLink.ts:7). A muted assets/one/talk.mp4 video and identically timed assets/two/talk.mp4 audio therefore enable Merge. The real menu/source-transform browser witness deleted the second asset's audio and unmuted the first video's different sound. Match normalized full asset identity, including source-file-relative resolution, before enabling or executing merge/link.

Blocker: packages/sdk/src/engine/linkedTiming.ts:22 — The group query descends into nested composition boundaries and addresses every match using the initiating scope prefix. Link ids are minted locally (lk-1 is normal in multiple files). An SDK witness with a root pair and an independent child.html pair both using lk-1 showed setTiming("hf-v", {start:2}) moving all four clips, including the child pair. Restrict membership to the resolved composition scope and retain scoped addresses. Audit the same raw-link-only matching in Studio's linkedMembersOf / expandToLinkedMembers and lint grouping.

Blocker: packages/studio/src/components/editor/mediaLinkEdits.ts:135 — Detach does not preserve an individual video's data-hidden mute on the inserted audio. A hidden video with sound produces an unhidden audio element: the actual runtime witness read gain 0 on the video and 1 on the detached audio. Detach must preserve the pre-edit audible mix; carry the mute state or refuse this case. The adjacent keep-sound cutout path uses the same sound-attribute list and needs the same audit.

CI: The Comments check fails at packages/lint/src/rules/media.ts:836: comment share increases from 116/1127 to 117/1131; the failure log explicitly identifies the added comment. Other observed checks pass.

Audited: media-link core helper, source detach/merge/link/unlink transforms, menu/shortcut/selection wiring, link plans and persistence, SDK timing mutation + undo integration, server split relinking, linked delete/unlink paths and permission boundaries, lint rule; traced gesture expansion into existing multi-move/group-trim commits and live/store/source identity.
Trusting: parent PRs and other owners' stack layers, unrelated timeline geometry/stacking implementation, full Studio/backend route and packaged render tests, full package suites.

Verification: Exact-head source/runtime/SDK witnesses reproduce all three blockers. 109 existing targeted tests passed at this exact head (10 files), plus 50 supplemental meter/render-dispatcher tests. Dependency collection failures were resolved using cached links and isolated targeted installs of cn@0.2.4 and @base-ui/react@1.7.0; no workspace-wide install. Detailed runs are in the accompanying evidence receipt; cached Vitest 3.2.4 is not declared 4.1.11. The local source-component browser fixture reproduces wrong-file merge and has a screenshot; it uses a local persistence adapter and is not full Studio/backend E2E. No prior reviews/comments on this PR.

— Magi

Verdict: REQUEST CHANGES
Reasoning: Merge can replace the wrong sound, timing edits can cross composition boundaries, and detach can resurrect muted audio; the comment gate also fails.

[github-actions]

Edit accuracy: 953 passing here, 953 on the base branch

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

• seqrepeat-none-px-r0-root-z100: tracking 0.05, pressJump 0, drop 0, reload 0, render 0.03, undo false, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.03, undo true, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.03, undo true, teleport true

[miguel-heygen]

Fix-delta review at 88255f82e0a26edf8279d72d84d35cde0cef96dc, against stacked parent a12e2f91b6cb88300ab4569a5bc25637438ba3af; compared with my previous review at dddeeddf.

Full asset paths now distinguish same-named files, and the source transform rechecks identity before deleting audio (packages/studio/src/components/editor/mediaLinkEdits.ts:162). Hidden state is retained by both detach and keep-sound cutout (:147, packages/studio/src/components/editor/mediaAudioEdits.ts:214). The previous separate-file SDK scope witness passes, and Comments is green.

Blocker — composition isolation is incomplete: packages/sdk/src/engine/linkedTiming.ts:15 only recognizes data-composition-file hosts through isNewHostBoundary; packages/studio/src/player/components/audioClipLink.ts:30 only compares sourceFile. Inline data-composition-id groups therefore remain joined, although the new lint scope rule/test explicitly accepts independently timed same-ID link groups in inline compositions.

Concrete witness: root video/audio at start 0, duration 4; inline child video/audio at start 5, duration 4; all data-link="lk-1", child has data-composition-id="child" and no file attribute. Lint reports no linked-clip finding. setTiming("hf-v", {start:2}) moves the child video from 5 to 2 too. Production Studio row construction gives all four sourceFile === undefined; linkedMembersOf(rootVideo, rows) returns all four instead of the root pair. Two independent expected-behavior assertions fail on this head. Preserve/use the actual composition identity consistently across SDK, Studio selection/edit expansion, unlinking and lint; add this inline-composition case alongside the separate-file case.

Verification / scope: 110 existing targeted OSS tests pass across loudness/menu, peaks (real FFmpeg), media edits, link groups/plans, SDK and lint on this exact head. The two additional inline-scope witnesses fail as above. Cached Vitest 3.2.4, worktree source aliases, happy-dom; no geometry, full Studio/backend, live playback or renderer acceptance. Audited repair files and their consumers; trusting unchanged parent implementations and unrelated main changes. Required captures still fail for missing Before/After evidence.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The full-path and hidden-audio repairs hold, but valid inline composition groups still cross-edit each other.

[jerrai-bot-heygen]

At 367fd1387, the nearest inline/file composition boundary repairs reused link-ID scoping across the inspected paths, but the SDK still converts a correctly scoped partner into an ambiguous bare element ID when the caller used a bare ID. Inline gives a child/root ID-collision that edits the wrong audio and the needed regression. Current-head checks pass apart from required Studio/player captures (missing Before/After). No local tests, build, merge or release. — Jerrai

[jerrai-bot-heygen]

The group is filtered to the right composition, but its members are then addressed with the caller's prefix. A bare ID can resolve to a unique video inside an inline composition (resolveScoped explicitly accepts that), while a partner's bare hf-a resolves to a top-level element with the same ID. With a child hf-cv/hf-a linked as lk-1 and a root hf-a, setTiming('hf-cv', { start: 2 }) changes the root audio and leaves the actual child partner unchanged; { linked: false } can unlink the wrong survivor as well (mutate.ts:290–307). Please derive each member's canonical scoped address from its resolved element (or reject ambiguous bare children) and add this duplicate-ID cross-scope regression. The existing reused-link-ID tests do not cover colliding element IDs.

[jerrai-bot-heygen]

At the exact #4817-based head, composition membership is scoped and linkedTiming.linkGroup refuses an address unless resolving it returns that member. The root/inline-child duplicate-ID regression asserts both linked and unlink paths refuse without mutation. Prior wrong-file merge and hidden-audio detach repairs remain in the 58-file layer patch; its three concurrent main edits preserve the layer patch, and captures pass. This closes my earlier code hold, not Miguel's independent review or CI/merge gates. — Jerrai

[jerrai-bot-heygen]

Exact b9930c73 rebase review: old and new parent→head comparisons retain the same 58 paths and per-file patches, with no concurrent parent edits on the layer paths. The scoped linked-timing lookup still verifies that the reconstructed address resolves to the intended member before edit; the duplicate root/child ID regression is retained. This is code approval for the layer, not clearance of #4815's red viewport check, pending CI, or Miguel's separately governing changes-requested review. No merge/release action. — Jerrai

[miguel-heygen]

Re-reviewed #4818 at b9930c73d6b356811bb44b94b999e89e96e04d9c against #4817 a0d266da2, additive to Jerrai's current approval.

The prior composition/address findings are fixed: nearest inline/file scope controls membership, and linkGroup checks that each address resolves to that member before timing or unlink mutation. Both paths gather partners before changing the target. The full-path and hidden-audio repairs remain present. Existing SDK, lint, link/source and menu/peak regressions pass in the combined export (65 cases); a further 17 menu cases exercise the lower #4816 versions.

Blocker — remote asset identity still loses the query (packages/studio/src/player/components/audioClipLink.ts:17, consumed by packages/studio/src/components/editor/mediaLinkEdits.ts:162–176). mediaAssetIdentity uses only origin and pathname. Thus a muted video at https://media.example/media.mp4?clip=first and audio at the same path with ?clip=second are treated as one asset even when that endpoint selects different media by query.

Independent reproduction at this exact head:

• A loopback HTTP endpoint served two real FFmpeg-generated MP4s with different audio, selected by those two query values. Their SHA-256 hashes differ, but the actual identity helper returns the same key and the actual Merge gate enables the pair.
• Two expected-refusal witnesses against the actual Studio helper and source transform fail. mergeAudioInSource deletes the second audio, removes muted from the first video and marks it as carrying audio. The source's second query-specific resource is lost.

Preserve identity-relevant URL query data for remote media, or refuse Merge/Link when identity is uncertain, and pin both the menu predicate and destructive source re-check. Project-local cache-buster normalization can remain a separate case. The source transform must not conclude equal assets from equal remote paths alone.

CI/captures pass; this is a distinct code hold. The HTTP and generated-media fixtures are synthetic and local; no customer media, full Studio/backend browser, live playback or paid render was used. Cached Vitest 3.2.4 differs from the declared version. Evidence is the real-media identity probe plus the two failing actual-source assertions.

Verdict: REQUEST CHANGES
Reasoning: the scoped-address repair holds, but the destructive merge can still replace a query-selected audio resource with a different video's sound. CI and the capture waiver do not address this reproduced asset-identity failure.

— Magi

[miguel-heygen]

Re-review at f8aedc5aaef551d323992d59ed75aeb9666cde66 against #4817 89fb3b8c1. The prior layer patch is preserved; the only new production change is remote-query retention. My original ?clip=first / ?clip=second case is now correctly refused.

Blocker — percent-encoded query data still collapses into query syntax. packages/studio/src/player/components/audioClipLink.ts:18-19 decodes the complete remote identity, including url.search. These two distinct URLs therefore compare equal:

• https://media.example/media.mp4?clip=first%26x=second (one clip value, first&x=second)
• https://media.example/media.mp4?clip=first&x=second (a clip value of first and a separate x parameter).

I served two actual FFmpeg-generated videos from one loopback path using those query forms. Their downloaded SHA256 hashes differ, but the exact-head helper equates them and findMergePair enables Merge. The actual mergeAudioInSource transform (mediaLinkEdits.ts:162-176) also accepts them, deletes the second audio element and unmutes the first video's different sound. Both expected-refusal witnesses fail on this head. This is the same wrong-resource/data-loss defect as the prior review, in an encoded-query case.

Keep remote origin/path/query in their serialized URL form rather than decoding reserved delimiters. Preserve the existing decoded project-local path normalization separately. In a temporary reviewer-only experiment, returning serialized origin/path/search for remote URLs makes all 43/43 targeted assertions pass, including local-query/hash normalization and the original case. No product patch was committed or pushed; the exact source was restored and the temporary export/server/media fixtures removed.

Current-head results before that experiment: 39 existing tests + 2 original witnesses pass; 2 encoded-query witnesses fail. Cached Vitest 3.2.4/source aliases, not a full Studio build or live playback. CI is still running with no currently reported failure; it does not cover this encoded distinction.

Verdict: REQUEST CHANGES
Reasoning: The new test covers different plain query values, but decoding remote query syntax still permits a destructive merge of different resources. Preserve serialized remote identity and add the encoded-query refusal regression.

— Magi

[miguel-heygen]

Re-review at faa911810388ddf8a976c591739bdacbe4b1a9a7. The plain-query and %26 query fixes both hold. The delta is the intended helper expression plus its regression assertion; the earlier scope/partner-address repairs remain unchanged.

Blocker — decoding a remote pathname still conflates a path character with URL syntax. packages/studio/src/player/components/audioClipLink.ts:19 now keeps the query serialized, but decodes url.pathname before concatenating them. Thus these URLs still have equal identities:

• https://media.example/media.mp4%3Fclip=first — the literal ?clip=first belongs to the pathname;
• https://media.example/media.mp4?clip=first — the pathname is media.mp4, with a query parameter.

I repeated the real-media loopback probe: these URLs served different FFmpeg-generated MP4 bytes (different SHA256), while the exact-head helper equated them and enabled Merge. Two tests using the actual findMergePair and mergeAudioInSource fail their expected refusal. The destructive transform (mediaLinkEdits.ts:162-176) deletes the second audio element and unmutes the first video's different sound. This is another reserved-delimiter instance of the same remote asset-identity defect.

Keep the remote pathname and search serialized, as in the branch tested in the previous review. Decode only the project-local pathname:

return url.origin === SOURCE_ORIGIN
  ? decodeURIComponent(`${url.origin}${url.pathname}`)
  : `${url.origin}${url.pathname}${url.search}`;

A temporary reviewer-only experiment with that branch passes 45/45 assertions: 39 existing helper/merge tests plus the original query, encoded-query and encoded-path witnesses. On the submitted head, 43 pass and the 2 encoded-path witnesses fail. The existing relative/local path, encoded-space and local-query/hash normalization tests continue to pass. No abstraction or dependency change is needed.

Verification uses cached Vitest 3.2.4 and actual source imports/aliases, not a full Studio build or live playback. CI remains in progress with no current failure. The test export, synthetic server and media files are removed; the experiment was restored, and no product source was committed or pushed.

Verdict: REQUEST CHANGES
Reasoning: The query now keeps its structure, but decoding remote path delimiters still allows destructive merging of distinct resources. Preserve serialized remote path/query and pin the encoded-path case too.

— Magi

[miguel-heygen]

Approved at d07611551a3b1bffc67ba6c4d5bc328e12d30fbc against #4817 89fb3b8c1. This supersedes my changes-requested review 5387670180 and the earlier remote identity holds.

audioClipLink.ts:18-19 now keeps remote origin/path/query serialized and confines decoding plus query/hash removal to the existing project-local branch. The helper is shared by Merge eligibility and its destructive source re-check, so both now refuse the distinct remote resources. The production delta is only this branch; the new assertion pins the encoded-path case. Previous composition/partner-address repairs are unchanged.

Local: 51/51 exact-source assertions pass: 39 existing helper/source-transform tests, the six original plain-query/encoded-query/encoded-path refusal witnesses, and six batched reserved-delimiter comparisons with Merge eligibility checks. This includes %2F, %3F, %23, encoded query-key =, and encoded query-value &; local relative paths, spaces, query/hash handling still pass. Both previously destructive transforms now refuse rather than deleting the different audio source.

Cached Vitest 3.2.4/source aliases rather than the declared 4.1.11; no full Studio build or live playback was rerun by me. Temporary source export and fixtures are removed, no product edits retained. Current CI has no failure but remains in progress; approval is not a merge-ready or release declaration.

Verdict: APPROVE
Reasoning: Serialized remote path and query preserve the delimiter distinctions in all reproduced failures, while the existing local normalization remains covered. No remaining blocker in this inspected repair.

— Magi

[miguel-heygen]

Re-approved at 09b046812ccc4beb521fd07ce8988fd5e07874cc against b99c7941333980c0cb0515f0e86e381a151cc25e, compared with my previously checked 83177626ebc2277a2fd205d558c1ced504893768 layer.

Audited: the complete 58-path parent-to-head patch and every layer file's blob. Paths and patch hunks are identical, ignoring object indexes/hunk positions; all file contents (source, tests and manifests) are byte-identical. Independent git range-diff marks all 21 commits =. The scoped partner-address and serialized remote path/query repairs remain unchanged.

Trusting / not rerun: the earlier focused tests and their documented environment limits; no new local tests, live Studio/playback or full package build. No new task worktree/server/browser was created. Current checks have no failure but are still running, so this is code approval rather than merge readiness. No merge, enqueue, release or CI bypass performed.

Verdict: APPROVE
Reasoning: The full rebased layer and its file contents match the previously reviewed layer, with no new code change or blocker.

— Magi