Skip to content

fix(studio): a split at a clip's edge leaves the file unchanged - #6826

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/split-at-edge-no-op
Oct 30, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/split-at-edge-no-op

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

A split at a clip's edge, or within the split margin (30 ms) of it, now leaves index.html unchanged. Before, splitting a clip several times at the same playhead could write a clip with data-duration="0".

Why

The cut is decided in splitElementInHtml, which refused only cuts at or beyond the exact edges (splitTime <= start || splitTime >= start + duration) and then wrote both halves rounded to the millisecond. A first split at a playhead of 2.3456 s writes the left half ending at 2.346 s. A second split at the same playhead then lands 0.0004 s inside that edge, passes the strict check, and the sliver rounds to a zero-length clip.

The timeline already refuses cuts within SPLIT_BOUNDARY_EPSILON_S of an edge, but against its in-memory copy of the clip, which can be one split behind the file. The server, which owns the file, never applied the margin.

Related work

None open: gh pr list --search "split in:title" shows only #5844, which is unrelated (text splitting).

How

  • SPLIT_BOUNDARY_EPSILON_S moves to the authored timing contract (parsers/compositionContract.ts, re-exported by @hyperframes/core/composition-contract), so the timeline and the server read one value. studio's timelineElementSplit re-exports it, so its importers are unchanged.
  • splitElementInHtml refuses a cut closer than the margin to either edge, inclusive at the margin, as the timeline's isSplitTimeWithinBounds is. Freeze frame's split goes through the same function and gets the same rule.

Test plan

  • Unit tests added: sourceMutationSplitAndGroup.test.ts, "a cut at an existing edge".
    • A cut at 2.3456, 2.346, 1, 1.0004 and 2.33 on a clip from 1 to 2.346 leaves the file byte-for-byte unchanged.
    • A cut exactly at the margin (1.03) still splits, into halves of 0.03 and 1.316.
  • studio-server: sourceMutationSplitAndGroup, freezeFrame and the files route tests, 178 passed. studio: timelineElementSplit and useRazorSplit, 25 passed.
  • tsc --noEmit clean in core, studio and studio-server; oxlint, oxfmt and fallow audit clean.
  • Manual testing in Studio: not driven. The rule is in the one function every split path writes through.
  • Comments say why: the margin's one-line doc names the rounding that made the zero-length half.

What I measured

fails without the fix: sourceMutationSplitAndGroup.test.ts › "a cut at an existing edge" › "leaves the file as it was when the cut is within the split margin of either edge". On main the 2.3456 s cut writes a half with data-duration="0".

No visible change

The fix is in the server's split. The studio change is a 2-line re-export of the margin, so the timeline looks and behaves as before.

Size

A lone fix: one guard where the file is cut, the margin moved to the contract both sides read, and its tests. No open PR on this path could carry it.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES · 3 blocker · 0 important · 0 nits · head 38813a49

The guard itself is right: the server now uses the same inclusive check as isSplitTimeWithinBounds, and every split path goes through it (Studio's single and split-all routes, freeze frame, CLI a2Shared). Before this merges:

  • The body has no fails without the fix: line. A fix PR has to name the test that fails on main on a line of its own, e.g. fails without the fix: sourceMutationSplitAndGroup.test.ts › "a cut at an existing edge" › leaves the file as it was…. Right now that claim sits only in the Test plan prose.
  • The Comments check fails because of this change. The comment ratchet flags +2 comment lines in each of compositionContract.ts and sourceMutation.ts (see the inline notes).
  • The Studio and player captures check fails. The PR touches packages/studio (a 2-line re-export) and the body has no ## Before/## After sections. This change has no visible effect and is under 20 lines with no .tsx/.css/.html, so a ## No visible change section is enough.
Scope

Read all 4 files plus the 4 callers of splitElementInHtml. Split-all now returns 400 for the whole batch when one cut is stale, and freeze frame returns null when a linked partner is within the margin. Both used to write a zero-length clip, so refusing is better. The float check is fine: the inclusive upper bound (start+duration-ε) leans toward allowing the cut, and the client computes it the same way. CI was still running other jobs (CI, CodeQL, preview/Windows render) when I posted this.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 91 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)

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

All 3 blockers answered at 28315d1:

  • The body now has its own fails without the fix: line, naming the test in sourceMutationSplitAndGroup.test.ts.
  • Comments: the 2 notes are fixed (replies inline). The check passes.
  • Studio and player captures: the body has a ## No visible change section. The check passes.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: APPROVE · 3 resolved, 0 open, 0 new · 38813a4 → 28315d1. I checked that the named test fails with main's sourceMutation.ts and passes here (51/51). All CI on this head is green, Windows included.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit b20d0a6 Oct 30, 2026
84 of 85 checks passed
@miguel-heygen
miguel-heygen deleted the fix/split-at-edge-no-op branch October 30, 2026 13:18
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.

1 participant