Skip to content

fix(studio): update unreferenced generated media names on replace - #5169

Open
miguel-heygen wants to merge 6 commits into
mainfrom
fix/studio-replaced-media-label
Open

miguel-heygen wants to merge 6 commits into
mainfrom
fix/studio-replaced-media-label

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Media replacement leaves a generated timeline name tied to the old source. The source writer now re-mints a media ID when it matches the old filename's generated slug, including collision suffixes, and the document has no references to it. IDs that do not match that generated slug, explicit labels, and referenced IDs stay unchanged. Runtime labels retain authored ID precedence.

The existing ID mint is shared by Studio insertion, SDK edits, and server edits. SDK forward and inverse patches include the ID change while preserving the stable edit identity. Reference checks cover scripts, CSS, other attributes, templates, escaped identifiers, and source URL fragments. Ordinary asset paths and identity definitions do not count as references.

Validation: replacement and decoding witnesses fail on main; the authored-ID cases fail against the earlier filename-preference implementation. Tests cover two replacements, collisions, undo, authored labels, and reference preservation. Removing the reference guard makes nine witnesses fail. Full lint, package-subpath verification, audit, formatting, and package/runtime typechecks pass locally.

Before

After replacement on the baseline, the generated name remains Harbor. Authored Hero Shot, referenced Marina, and explicit Opening Shot are preserved.

Before media replacement naming

After

Real HTTP edits through Studio's source writer and preview reload change Harbor to Library. The authored and referenced names remain unchanged.

After first media replacement

A second replacement updates Library to Sunset through the same stable edit target.

After second media replacement

Known baseline gate red: Studio: timeline viewport gate, the overscan regression tracked in #5151.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 7, 2026 07:41

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 22ad7a13.

The filename decoding is careful and well tested, and the stale "Harbor" after a replace is a real bug. My concern is with how the fix gets there: it moves the filename ahead of the element id for every video, audio and image clip (timeline.ts:237-240), not only clips whose id Studio generated. So hand-written ids lose their labels too.

Blocker: authored media ids get renamed. In HyperFrames the id is often the name the author picked, especially in agent-written HTML. I ran it in the runtime test environment:

<video id="a-roll" src="video.mp4" data-start="0" data-duration="20"></video>
<audio id="voiceover" src="assets/tts_8f3a2c.mp3" data-start="0" data-duration="10"></audio>
  • main: ["A Roll", "Voiceover"]
  • this head: ["Video", "Tts 8f3a2c"]

That exact a-roll <- video.mp4 / a-roll <- source.mp4 shape is in our own skills/hyperframes-registry/references/wiring-blocks.md:11 and skills/embedded-captions/scripts/make-theme.cjs:583. So the PR body's "a name you typed yourself stays" holds for data-timeline-label / data-label / aria-label, but not for an id typed by hand.

There is also a consistency gap. Studio's own DOM label path still puts the id first (getTimelineElementDisplayLabel, used by parseTimelineFromDOM and the standalone-root fallback). After this change, the two paths name the same clip differently.

Suggested direction (smaller, and fixes the cause): the stale label only happens because Studio's drop generates the id from the filename (buildTimelineAssetId in studio/src/utils/timelineAssetDrop.ts). Then a replace rewrites src but has to keep the id. The Studio write that sets the new src knows the old one. If the id is what buildTimelineAssetId gives for the old file (its slug, optionally _N), the name was generated, and that write can name the clip after the new file. Leave the runtime's label order as it is. Hand-authored ids keep their names, and both label paths stay in agreement. If you'd rather keep the runtime change, it needs a way to tell a generated id from an authored one. Unconditional filename-first is the part I'm objecting to.

Minor: if the runtime change stays, the later filenameFromAssetUrl fallback (timeline.ts:251-252) is now only reachable for non-media kinds. Worth folding into the one branch.

Tests: src/runtime/timeline.test.ts passes 81/81 at this head.

Verdict: REQUEST CHANGES
Reasoning: The fix repairs Studio-generated ids by renaming every media clip after its file, which changes labels authors chose (a-roll becomes "Video"), and that case appears in our own skills.

— Rames

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1589 of those

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)

Comment thread packages/core/src/timelineAssetId.ts Fixed
@miguel-heygen miguel-heygen changed the title fix(core): keep generated media labels current fix(studio): update unreferenced generated media names on replace Oct 7, 2026
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Addressed at 50e8d82. Runtime labels retain authored ID precedence. The source writer now re-mints only an unreferenced ID matching the old source’s generated slug (including collision suffixes); explicit labels and other IDs remain unchanged. Studio insertion, SDK edits, and server edits share the existing mint and replacement rule. Regression coverage includes authored hero-shot, two replacements, undo, and document references. Full lint, audit, package/runtime typechecks, and focused tests pass. The refreshed Before/After captures show real Studio edits and preview reloads on this head, with Library then Sunset while the authored and referenced names stay fixed.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 50e8d826. My blocker from 22ad7a13 is resolved, so I'm approving.

Prior blocker: authored ids were renamed by filename-first labels. timeline.ts no longer puts the filename ahead of the id. The runtime change is now only the filename decoding, so an authored a-roll on video.mp4 keeps its label. The rename moved to the source writer, where it belongs. replacementTimelineAssetId re-mints the id only when all of these hold:

  • the id equals the old file's generated slug, or the slug plus a collision suffix;
  • there's no explicit label;
  • nothing in the document references the id.

That's the shape I suggested, and it reuses the existing buildTimelineAssetId, now moved to core instead of copied.

What I checked

  • Both persistence paths are covered. The SDK cutover path is handleSetAttribute in mutate.ts, and the server patch path is patchElementInHtml in sourceMutation.ts. Those are the two branches in useDomEditPersist, so every src write from Studio goes through one of them, including the new img Fill write from #5170.
  • The SDK keys its patch paths by hf-id (attrPath(id, …) with id: HfId), and the server stamps hf-ids into every composition. So the selection still resolves after the id changes, and the id change rides in the same forward and inverse patch for undo.
  • The reference check is deliberately loose: any substring hit in a script, a style or another attribute blocks the rename. A false positive only means no rename, which fails safe.
  • Locally, after building core for the new ./timeline-asset-id export, these pass: core timelineAssetId and timelineMediaLabels (12), sdk mutate.test.ts (114), studio-server sourceMutation.test.ts (71) and studio timelineAssetDrop (34).

Notes (non-blocking)

  1. The reference check only sees the edited document. An id used only from another file is invisible to it, so the rename would silently orphan those references. Two cases: a parent composition's script targeting #harbor inside a sub-composition, or an external script such as the <script src="assets/glass-main.js"> kind some registry examples use. The exposure is small, because the id has to be Studio-generated and then hand-referenced only from elsewhere. It's worth a comment at replacementTimelineAssetId saying the check is single-document.
  2. An authored id that happens to equal the file's slug (<video id="intro" src="intro.mp4">) is treated as generated and follows the new file. That's indistinguishable from a Studio drop, and following the file is a reasonable outcome. I'm noting it so it's a known choice.
  3. Nit: in mutate.ts, the new import sits above the file's doc comment.

Verdict: APPROVE
Reasoning: The authored-id regression is gone because the label precedence is unchanged and the rename is limited to provably generated, unreferenced ids, applied in both writers and covered by tests.

— Rames

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants