Skip to content

fix(studio): warn on anonymous timeline clips#533

Open
miguel-heygen wants to merge 2 commits intomainfrom
fix/studio-editable-id-warnings
Open

fix(studio): warn on anonymous timeline clips#533
miguel-heygen wants to merge 2 commits intomainfrom
fix/studio-editable-id-warnings

Conversation

@miguel-heygen
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen commented Apr 28, 2026

Problem

Studio timeline editing still had two rough edges that made the latest alpha feel less polished when testing it like a video editor would:

  • Timeline clips for anonymous DOM nodes could surface internal fallback identities like __node__index_*, which made the timeline look broken instead of authored.
  • Elements without a stable id could still appear in the timeline and canvas editor, but authors did not get direct lint guidance that those elements are weaker targets for Studio and agent edits.

What this fixes

  • Adds a non-blocking studio_missing_editable_id lint warning for timeline-visible elements that do not have an id.
  • Makes the warning point to the exact element and recommend stable, human-readable ids such as hero-title or scene-1-card.
  • Stops using synthetic node-index ids as runtime clip identity for anonymous DOM nodes.
  • Gives anonymous clips readable labels from authored metadata, composition ids, DOM ids, class names, asset filenames, text content, or a simple ordinal fallback.
  • Keeps those labels display-only in Studio and uses key-first identity for matching, dragging, resizing, and manifest merge preservation.
  • Covers the duplicate-label case where two anonymous clips both render as Card but still stay separate timeline entries.

Root cause

The runtime manifest used synthetic node-index ids as both identity and display fallback for timeline nodes that had no stable author-provided id. Studio then treated those internal values as user-facing clip names.

The first pass improved the display label, but it also risked using that friendly label as internal identity. Two anonymous clips with the same label could then collapse into the same logical timeline element. The fix separates display labels from internal identity and prefers the timeline key whenever Studio needs to match an element.

The linter also had correctness checks for render and runtime behavior, but it did not teach authors when a timeline-visible element would be harder for Studio and agents to patch reliably. That left missing ids as a silent authoring quality issue instead of actionable guidance.

Verification

Local checks

  • bun run --cwd packages/core test -- src/lint/rules/core.test.ts src/runtime/timeline.test.ts -> 41 tests pass
  • bun run --cwd packages/studio test -- src/player/hooks/useTimelinePlayer.test.ts src/player/components/timelineTheme.test.ts -> 23 tests pass
  • bun run --cwd packages/core typecheck
  • bun run --cwd packages/studio typecheck
  • bun run --cwd packages/studio build -> passes with the existing Vite chunk-size warning
  • bunx oxlint $(git diff --name-only origin/main...HEAD) -> 0 warnings, 0 errors
  • bunx oxfmt --check $(git diff --name-only origin/main...HEAD)
  • git diff --check origin/main...HEAD

Browser verification

  • Created a scratch project at /tmp/hf-pr533-conflict-verify with two timed anonymous .card clips that both label as Card.
  • Started the local Studio dev server for pr-533-conflict-verify.
  • Used agent-browser to verify the timeline renders two separate Card clips instead of collapsing duplicate anonymous labels.
  • Used agent-browser to open the Studio lint modal and verify it shows human-readable missing-id warnings, not internal node-index labels.
  • Used agent-browser to click Play after the lint pass and confirm the timeline remains usable.
  • Recorded the tested Studio flow with agent-browser.

Notes

  • Rebased onto current main; conflict resolution preserved both the newer mainline Studio shortcut/lint behavior and this PR's anonymous-clip identity split.
  • GitHub Actions are running on the rebased head.
  • Scratch verification files are intentionally not committed.
  • Local screenshots and recording from this rebase pass are under .codex-artifacts/pr-533-conflict-rebase-2026-04-29/.

@miguel-heygen miguel-heygen marked this pull request as ready for review April 28, 2026 19:50
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

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

Staff review: requesting changes.

The display-label fix is the right direction, but this still conflates user-facing labels with internal timeline identity. In packages/studio/src/player/hooks/useTimelinePlayer.ts, anonymous clips now flow through id = clip.id || label and id = el.id || compId || label. That means common timelines with two anonymous clips sharing the same class/label, for example two .card clips, can produce duplicate TimelineElement.id values.

That is risky because other Studio paths still treat id as identity: mergeTimelineElementsPreservingDowngrades builds a Set from element.id, selection/update paths fall back to key ?? id, and several edit/error paths still report or compare element.id. The new label field solves the user-facing name problem, so the internal identity should stay unique or all identity-sensitive paths need to be key-first throughout.

Please add coverage with two anonymous clips that generate the same friendly label and verify they remain distinct through runtime-manifest sync/fallback parsing and are not merged, dropped, or updated together.

@miguel-heygen miguel-heygen force-pushed the fix/studio-editable-id-warnings branch from 788ed66 to e4fd0d7 Compare April 28, 2026 20:54
@miguel-heygen miguel-heygen changed the base branch from next to main April 28, 2026 20:55
@miguel-heygen
Copy link
Copy Markdown
Collaborator Author

@vanceingalls all was addressed btw.

@miguel-heygen miguel-heygen force-pushed the fix/studio-editable-id-warnings branch from e4fd0d7 to 90e9027 Compare April 29, 2026 20:56
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.

2 participants