From 8cc7292f134e2f493437390f3be01f4b04735eb6 Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Sat, 13 Jun 2026 01:20:59 -0300 Subject: [PATCH] fix(studio): apply split-bounds epsilon in razor split-all The single-clip razor path guards splits with isSplitTimeWithinBounds, which keeps a SPLIT_BOUNDARY_EPSILON_S margin from each clip edge so a cut never produces a degenerate near-zero slice. The split-all path filtered with raw `splitTime > start && splitTime < end` instead, so it accepted cuts inside that margin (and on clips shorter than two epsilons that the single path always rejects), producing the very degenerate slice the epsilon exists to prevent. Extract the shared predicate canSplitElementAt and a selectSplittableElements helper, and route both razor paths through them so the two stay consistent. Adds unit coverage for the new helpers, including the regression where a sub-epsilon clip with an interior split time must not be selected. --- packages/studio/src/hooks/useRazorSplit.ts | 14 ++--- .../src/utils/timelineElementSplit.test.ts | 62 ++++++++++++++++++- .../studio/src/utils/timelineElementSplit.ts | 18 ++++++ 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/packages/studio/src/hooks/useRazorSplit.ts b/packages/studio/src/hooks/useRazorSplit.ts index 19dfad0b9..369d8b012 100644 --- a/packages/studio/src/hooks/useRazorSplit.ts +++ b/packages/studio/src/hooks/useRazorSplit.ts @@ -4,10 +4,10 @@ import { usePlayerStore } from "../player"; import { saveProjectFilesWithHistory } from "../utils/studioFileHistory"; import { getTimelineElementLabel, collectHtmlIds } from "../utils/studioHelpers"; import { - canSplitElement, + canSplitElementAt, + selectSplittableElements, buildPatchTarget, readFileContent, - isSplitTimeWithinBounds, } from "../utils/timelineElementSplit"; import type { RecordEditInput } from "./useTimelineEditing"; @@ -169,11 +169,7 @@ export function useRazorSplit({ } const pid = projectIdRef.current; - if (!pid || !canSplitElement(element)) return; - - if (!isSplitTimeWithinBounds(splitTime, element.start, element.duration)) { - return; - } + if (!pid || !canSplitElementAt(element, splitTime)) return; try { const { targetPath, originalContent, patchedContent, changed, skippedSelectors } = @@ -230,9 +226,7 @@ export function useRazorSplit({ const pid = projectIdRef.current; if (!pid) return; const { elements } = usePlayerStore.getState(); - const splittable = elements.filter( - (el) => canSplitElement(el) && splitTime > el.start && splitTime < el.start + el.duration, - ); + const splittable = selectSplittableElements(elements, splitTime); if (splittable.length === 0) return; try { diff --git a/packages/studio/src/utils/timelineElementSplit.test.ts b/packages/studio/src/utils/timelineElementSplit.test.ts index fdb0913d4..d5262dd8e 100644 --- a/packages/studio/src/utils/timelineElementSplit.test.ts +++ b/packages/studio/src/utils/timelineElementSplit.test.ts @@ -1,5 +1,22 @@ import { describe, it, expect } from "vitest"; -import { SPLIT_BOUNDARY_EPSILON_S, isSplitTimeWithinBounds } from "./timelineElementSplit"; +import type { TimelineElement } from "../player/store/playerStore"; +import { + SPLIT_BOUNDARY_EPSILON_S, + canSplitElementAt, + isSplitTimeWithinBounds, + selectSplittableElements, +} from "./timelineElementSplit"; + +function element(overrides: Partial = {}): TimelineElement { + return { + id: "el-1", + tag: "div", + start: 1, + duration: 4, + track: 0, + ...overrides, + }; +} describe("isSplitTimeWithinBounds", () => { const start = 1; @@ -48,3 +65,46 @@ describe("isSplitTimeWithinBounds", () => { expect(isSplitTimeWithinBounds(start + shortDuration / 2, start, shortDuration)).toBe(false); }); }); + +describe("canSplitElementAt", () => { + it("accepts a splittable element at an interior time", () => { + expect(canSplitElementAt(element({ start: 1, duration: 4 }), 3)).toBe(true); + }); + + it("rejects a time inside the boundary epsilon", () => { + expect( + canSplitElementAt(element({ start: 1, duration: 4 }), 1 + SPLIT_BOUNDARY_EPSILON_S / 2), + ).toBe(false); + }); + + it("rejects locked, implicit and sub-composition elements", () => { + expect(canSplitElementAt(element({ timelineLocked: true }), 3)).toBe(false); + expect(canSplitElementAt(element({ timingSource: "implicit" }), 3)).toBe(false); + expect(canSplitElementAt(element({ compositionSrc: "child.html" }), 3)).toBe(false); + }); +}); + +describe("selectSplittableElements", () => { + it("excludes a clip shorter than two epsilons even when the time is inside it", () => { + // Regression: split-all used raw start < t < end, so a clip too short for + // the epsilon margin was still selected and produced a degenerate slice. + const tiny = element({ id: "tiny", start: 1, duration: SPLIT_BOUNDARY_EPSILON_S + 0.01 }); + const interiorTime = tiny.start + tiny.duration / 2; + expect(interiorTime).toBeGreaterThan(tiny.start); + expect(interiorTime).toBeLessThan(tiny.start + tiny.duration); + expect(selectSplittableElements([tiny], interiorTime)).toEqual([]); + }); + + it("keeps only the elements whose epsilon-bounded range contains the time", () => { + const inside = element({ id: "inside", start: 0, duration: 4 }); + const outside = element({ id: "outside", start: 5, duration: 4 }); + const locked = element({ id: "locked", start: 0, duration: 4, timelineLocked: true }); + const result = selectSplittableElements([inside, outside, locked], 2); + expect(result.map((el) => el.id)).toEqual(["inside"]); + }); + + it("accepts an element at the exact lower clamp boundary", () => { + const el = element({ start: 2, duration: 4 }); + expect(selectSplittableElements([el], 2 + SPLIT_BOUNDARY_EPSILON_S)).toEqual([el]); + }); +}); diff --git a/packages/studio/src/utils/timelineElementSplit.ts b/packages/studio/src/utils/timelineElementSplit.ts index 4a3454ff1..cb5521e25 100644 --- a/packages/studio/src/utils/timelineElementSplit.ts +++ b/packages/studio/src/utils/timelineElementSplit.ts @@ -30,3 +30,21 @@ export function canSplitElement(el: TimelineElement): boolean { Number.isFinite(el.duration) ); } + +/** + * True when `el` can be split AND `splitTime` lies within its boundary epsilon. + * Shared by the single-clip and split-all razor paths so both honor the same + * minimum-distance rule (split-all previously used raw `>`/`<`, letting cuts + * land inside the epsilon margin and produce a degenerate slice). + */ +export function canSplitElementAt(el: TimelineElement, splitTime: number): boolean { + return canSplitElement(el) && isSplitTimeWithinBounds(splitTime, el.start, el.duration); +} + +/** Elements that the split-all razor action can cut at `splitTime`. */ +export function selectSplittableElements( + elements: TimelineElement[], + splitTime: number, +): TimelineElement[] { + return elements.filter((el) => canSplitElementAt(el, splitTime)); +}