Skip to content

fix(cli): preserve authored clip length when moving - #5315

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
rudycelekli:fix/timeline-move-authored-duration
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
rudycelekli:fix/timeline-move-authored-duration

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

What

Preserve an end-authored clip's resolved length when moving it with the timeline CLI.

Why

The timeline reader supports older clips with data-start and data-end. The move command checked bounds and overlaps using the original length but only changed the start attribute. A two-second clip at 1–3 seconds moved to 2 seconds became one second long; moving a two-second clip at 4–6 seconds to 1 second made it five seconds long. The same incorrect result appeared in --plan.

Related work

This is a small correction to the existing timeline move command introduced in #4221. Open #5279 concerns atomic batch undo receipts; its implementation is independent, although both PRs append CLI regressions to the same test file.

How

For a clip with an authored duration, write its already resolved duration and remove the derived/legacy end attribute alongside the new start. This follows the existing canonical timing writer's contract. Clips whose duration comes from natural media length or an image default retain their unauthored duration.

Test plan

  • Eight public CLI regressions added: forward/backward end-authored moves and duration-authored/default-image controls, each in applied and planned modes. Four assertions fail before the fix; all 49 timeline cases pass afterward. Applied results are reinspected from persisted source, and planned source bytes remain unchanged.
  • Built package CLI under Node 22.19.0: four independent forward/backward applied/planned cases reproduce the original duration change and preserve two seconds afterward.
  • Frozen Bun 1.4.2 install, full workspace build, CLI rebuild/typecheck, formatting/lint, test reachability, comment checks, and normal signed commit hooks pass. No comments were added.
  • Documentation: the existing canonical timing contract remains applicable.

The full CLI suite reports 4594 passed, 6 failed, 49 skipped out of 4649 collected tests. An unchanged-production comparison with the same tests and environment reports 4590 passed, 10 failed, 49 skipped, including the four timing regressions. Five exact failures match between runs: the history held-lock test, the atomic file watcher test, and three Sherpa installation tests. The sixth failure is in the unchanged real-filesystem watcher module: the new-directory case fails after the fix, while the manifest-write case fails in the comparison. That module has no dependency on the changed timeline helper. The full CLI suite is not reported as passing.

Validation covers CLI timing and persistence on macOS. Hosted upstream CI and maintainer review remain outstanding.

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>

@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.

Approving. Reviewed at daab5e74.

The bug is still on main. moveMutation checks bounds with the resolved row.duration but only rewrites data-start, so a legacy data-start/data-end clip keeps its old end and changes length. Writing the resolved duration and removing data-end follows the contract in compositionContract.ts (writers emit start + duration, data-end is derived). It also fixes a quieter case: a duration-authored clip carrying a compiler-derived data-end used to end up with a conflicting stale end after a move. Clips whose length comes from the media or the image default are left alone because durationAuthored is false for them, and the image control case covers that.

On a local merge with current main, after a full build, timeline.e2e.test.ts passes 68 of 68. With the old a2Shared.ts put back, exactly the four end-authored cases fail (applied and --plan, both directions).

A non-blocking note on the same pattern next door: trimMutation also writes data-start and data-duration without clearing data-end. Its length comes out right because the canonical duration wins, but a trimmed legacy clip is left with a conflicting data-end that lint reports. Clearing it there too, or sharing a small "write canonical timing" helper between the two, would close the whole class. Fine as a follow-up.

Need: yes. Size: ok. Reuse: follows the existing canonical timing contract. Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 004409e Oct 24, 2026
81 checks passed
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