Repository navigation
fix(studio): a split at a clip's edge leaves the file unchanged - #6826
Conversation
There was a problem hiding this comment.
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 onmainon 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
Commentscheck fails because of this change. The comment ratchet flags +2 comment lines in each ofcompositionContract.tsandsourceMutation.ts(see the inline notes). - The
Studio and player capturescheck fails. The PR touchespackages/studio(a 2-line re-export) and the body has no## Before/## Aftersections. This change has no visible effect and is under 20 lines with no .tsx/.css/.html, so a## No visible changesection 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.
Edit accuracy: accurate 271 (base branch 271), smooth 91 of thoseThe gate passes. Quarantined, measured but not gated (0) |
|
All 3 blockers answered at 28315d1:
|
What
A split at a clip's edge, or within the split margin (30 ms) of it, now leaves
index.htmlunchanged. Before, splitting a clip several times at the same playhead could write a clip withdata-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_Sof 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_Smoves 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'stimelineElementSplitre-exports it, so its importers are unchanged.splitElementInHtmlrefuses a cut closer than the margin to either edge, inclusive at the margin, as the timeline'sisSplitTimeWithinBoundsis. Freeze frame's split goes through the same function and gets the same rule.Test plan
sourceMutationSplitAndGroup.test.ts, "a cut at an existing edge".studio-server:sourceMutationSplitAndGroup,freezeFrameand thefilesroute tests, 178 passed.studio:timelineElementSplitanduseRazorSplit, 25 passed.tsc --noEmitclean incore,studioandstudio-server;oxlint,oxfmtandfallow auditclean.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". Onmainthe 2.3456 s cut writes a half withdata-duration="0".No visible change
The fix is in the server's split. The
studiochange 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.