feat(studio): razor/blade tool UI for timeline clip splitting

[miguel-heygen]

Summary

Wire the razor tool into Studio's timeline UI — the standard NLE workflow for splitting clips at arbitrary positions.

• B enters razor mode (crosshair cursor + red vertical guide line)
• Click any clip to split at the click position (not the playhead)
• Shift+click splits all clips across every track at that time
• V or Escape exits razor mode
• Toolbar shows selection arrow / scissors toggle

Adds useRazorSplit hook for split orchestration. Adds activeTool state to playerStore. Adds preview reload after timeline move/resize operations.

Test plan

• Tested with 10-element composition covering gsap.to, fromTo, keyframes, stagger, multi-track, locked elements
• Split elements visible with correct CSS and GSAP state
• Undo works
• Locked elements rejected

Stack

1/3 — #1329 (cleanup)
2/3 — #1330 (engine)
3/3 — this PR (UI)

[miguel-heygen]

• feat(studio): razor/blade tool UI for timeline clip splitting #1331 👈 (View in Graphite)
• feat(core): GSAP-aware split engine for timeline clip splitting #1330
• refactor(studio): extract shared timeline components and deduplicate code #1329
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Covering canonical rubric only; deferring HF-domain (blade tool UX, Adobe Premiere parity, click-to-split frame snapping, runtime preview correctness) to Vai.

Two blockers + two concerns + a few nits.

Blockers
• lefthook.yml:25-29 — adds --health-baseline .fallow/health-baseline.json, --dupes-baseline .fallow/dupes-baseline.json, --dead-code-baseline .fallow/dead-code-baseline.json to the pre-commit hook, but .fallow/ does not exist on this PR branch OR on main, and is not gitignored. The pre-commit hook will break for every committer after this lands until the baselines are added. Either land the baseline JSON files in this PR (or a sibling), or drop the flags. Verified by gh api .../contents/.fallow?ref=feat/razor-blade-ui → 404 and ?ref=main → 404.
• packages/studio/src/hooks/useRazorSplit.ts:447 — handleRazorSplit (which now also backs handleTimelineElementSplit per the export rewiring at useTimelineEditing.ts:684) drops two behaviors from the old handleTimelineElementSplit:

1. The isRecordingRef?.current guard. The old code at useTimelineEditing.ts:576-580 showed "Cannot edit timeline while recording" and bailed. The new hook doesn't take isRecordingRef at all, so S-key splits and B-key razor clicks both go through during recording.
2. The data.changed check. Old code at useTimelineEditing.ts:634-637: if (!data.ok || !data.changed) { showToast("Failed to split clip — playhead may be outside the clip.", "error"); return; }. New splitHtmlElement in useRazorSplit.ts:351-368 only checks !response.ok and proceeds when the server returned { ok: true, changed: false } — writing originalContent back to disk, reloading preview, and showing a misleading "Split <label> at Xs" success toast.
   Both are user-visible regressions on a hot path. The isRecordingRef one is the more concerning of the two.

Concerns
• packages/studio/src/player/components/Timeline.tsx:812-822 — onPointerDown Shift+razor branch reads scrollRef.current.getBoundingClientRect() and subtracts GUTTER to compute splitTime. Fine for the gutter, but: the parent <div ref=setContainerRef> also has an onMouseMove that sets razorGuideX without subtracting GUTTER (line 800). So the red guide line drawn at razorGuideX and the actual split-time computed on click use two different x-origins. Visual/behavioral mismatch — the guide and the cut may not line up at the gutter boundary. Worth checking against the Loom; Vai will likely see this too.
• packages/studio/src/hooks/useRazorSplit.ts:491-503 — handleRazorSplitAll runs N splits sequentially with await handleRazorSplit(element, splitTime) per clip. Each call hits two API endpoints (file-mutations/split-element + gsap-mutations) and does its own saveProjectFilesWithHistory + reloadPreview. For a 5-track shift+click, that's 10 network round-trips and 5 preview reloads — visible UI stall. Also: each iteration's saveProjectFilesWithHistory reads originalContent from disk, but the previous iteration just wrote a different file — race-window potential on the project state if two splits target the same file. Worth batching or at minimum suppressing intermediate reloadPreview calls.

Nits
• packages/studio/src/player/components/TimelineCanvas.tsx:893 — const epsilon = 0.03; is a magic constant without a comment. Why 30ms? Frame-boundary related? Comment justification keeps Future Reader honest. (nit)
• packages/studio/src/components/TimelineToolbar.tsx:86 — extra blank line between AutoKeyframeToggle and DomEditSessionSlice. (nit)
• packages/studio/src/components/TimelineToolbar.tsx:149 — inline SVG <path d="M2 0.5L10 6L6.5 6.5L8.5 11L6.5 11.5L4.5 7L2 9Z" /> for the selection-arrow icon. Worth lifting into src/icons/SystemIcons next to Scissors for consistency. (nit)
• packages/studio/src/hooks/useRazorSplit.ts:1 — no useRazorSplit.test.ts. PR adds 183 LOC of new hook logic (epsilon clamp, generateSplitId collision dedup, executeSplit orchestration, sequential split-all) and has zero automated tests. PR body lists manual smoke testing on a 10-element comp. Canonical rubric flags this as thin for the diff size; not a blocker on its own since the underlying split engine (#1330) is well-tested. (concern-leaning nit)

What I didn't verify (Vai's lane)
• Click-to-split position correctness (where exactly the cut lands relative to playhead/cursor) against Adobe Premiere parity.
• Razor cursor / guide line UX vs Loom expectations.
• Preview reload semantics after split (does the runtime see both halves on the right frame?).
• Whether the 0.03s epsilon at clip boundaries matches frame snapping at 30fps / 60fps / etc.

— Rames D Jusso

[vanceingalls]

The UI work here is well-scoped (toolbar toggle, crosshair cursor, click-to-split, B/V/Escape hotkeys, shift+click for split-all). The new useRazorSplit hook is the right abstraction. But there are three real regressions in this PR independent of the engine concerns I called out on #1330, and the recording/UX regressions in particular shouldn't ship. Verdict: REQUEST_CHANGES.

Blockers

1. reloadPreview() after every move and resize is a perf regression that fights an existing optimization.

useTimelineEditing.ts:137,173 now call reloadPreview() after enqueueEdit in both handleTimelineElementMove and handleTimelineElementResize. reloadPreview() in App.tsx:136 bumps refreshKey, which (via NLELayout.tsx:159-164) calls refreshPlayer(), which sets iframe.src with a cache-busting _t param (useTimelinePlayer.ts:477-485). That is a full iframe reload.

The existing comment in timelineEditingHelpers.ts:41-43 documents the design:

"The runtime re-reads data-start/data-duration from the DOM on each sync tick (packages/core/src/runtime/init.ts:1324-1368), so attribute mutations here are picked up automatically on the next frame without a rebind call."

Adding a full reload on every drag-end nullifies this. For a video editor doing rapid timeline edits — the exact target user of this feature — every drag now causes a visible iframe flash and (if any sub-comp HTTP fetches happen during init) a multi-hundred-ms hitch. This is a regression.

The justification I can guess at: after a SPLIT, the runtime does need a reload because new DOM elements and new GSAP tweens won't be picked up by attr-polling alone. But that reasoning doesn't extend to move/resize, which are pure attribute mutations.

Fix: call reloadPreview() only inside useRazorSplit.ts (where it's already there at line 150) and remove it from handleTimelineElementMove/handleTimelineElementResize. If there's a deeper reason for the reload — e.g., the iframe drifts out of sync with data-track-index changes — fix the runtime to poll those too, or add it only in the cases where it's actually needed.

2. Recording guard is lost on razor splits.

useTimelineEditing.ts (master) gates every other mutation through enqueueEdit, which checks isRecordingRef?.current at line 84 and refuses to edit. The pre-PR handleTimelineElementSplit had the same guard inline (line 391-394 of the old code, removed in this PR). The new handleRazorSplit in useRazorSplit.ts has no recording check at all — it doesn't even receive isRecordingRef as a prop. Users can now razor-split clips during an active gesture recording, corrupting the recorded state. Restore the guard.

3. UX regression: razor split silently fails on every error path.

The pre-PR handleTimelineElementSplit surfaced three failure reasons via toast:

• "Cannot edit timeline while recording"
• "Playhead must be inside the clip to split."
• "Clip is missing a patchable target."

useRazorSplit.handleRazorSplit replaces these with bare returns (lines 128-129):

if (!pid || !canSplitElement(element)) return;
if (splitTime <= element.start || splitTime >= element.start + element.duration) return;

For a razor tool — where the user clicks the clip expecting it to split — a silent no-op is the wrong UX. The user clicks, nothing happens, they don't know why. The B-key flow is even more confusing: enter razor mode, click on a locked clip, no feedback. Surface the existing reasons via toast. The element-out-of-range case is mostly handled by the UI epsilon clamp in TimelineCanvas.tsx:393, but canSplitElement failures (locked, implicit timing, composition) need a user-facing reason.

4. clampedTime uses previewElement.start, but the click target is the actual element.

TimelineCanvas.tsx:386-401 computes splitTime from previewElement.start + clickOffsetX / pps — previewElement is the drag-preview clone (its start is the drag-preview start, not the persisted start). Outside of a drag, previewElement === el, but during drag (where the preview is offset from the persisted position), the split coordinate refers to a position that doesn't match where the clip actually rests on disk. Since the click handler shouldn't fire during a drag (drag suppresses click), this is probably not exploitable — but the variable shadowing makes the code brittle and it's easy to break. Use el.start + clickOffsetX / pps for clarity, or assert that previewElement === el at click time.

Important

5. handleRazorSplitAll is sequential and produces N history entries instead of one.

Lines 167-180: the for-loop awaits each handleRazorSplit in turn, each of which performs an HTTP round-trip to /file-mutations/split-element, another to /gsap-mutations, and then saveProjectFilesWithHistory. On a composition with 12 tracks (per the PR's test plan with 10 elements), shift+click triggers 12 sequential saves and 12 history entries. Undo now requires 12 Ctrl-Z presses to revert a single shift-click. Either coalesce into one history entry (one recordEdit call with all 12 file diffs aggregated), or wrap the whole operation in a single saveProjectFilesWithHistory call with all [targetPath]: patchedContent merged.

6. Race: handleRazorSplitAll may corrupt itself.

Each handleRazorSplit call re-fetches the original content with readFileContent. After clip 1 is split, the disk is written. Then clip 2's split starts: it reads the disk (now includes clip 1's split), splits clip 2, writes again. So far OK. But the originalContent passed to saveProjectFilesWithHistory for clip 2 is the post-clip-1 content — meaning if undo restores originalContent, it restores the state after clip 1 was split. Each undo only rewinds one split. That's consistent with the N-history-entries problem above, so the same coalescing fix solves it.

Also: usePlayerStore.getState().elements is read once at the start of handleRazorSplitAll, but the loop awaits between iterations. If the store updates mid-loop (the player re-syncs after the iframe reload triggered by each handleRazorSplit), the second/third iteration is operating on stale element references. The split still works because we look up by domId server-side, but it's a latent footgun.

7. lefthook.yml references baseline files (.fallow/health-baseline.json etc.) that aren't included in the PR.

The diff adds:

--health-baseline .fallow/health-baseline.json
--dupes-baseline .fallow/dupes-baseline.json
--dead-code-baseline .fallow/dead-code-baseline.json

These files aren't in the repo on this branch. Either they need to land in this PR, or there's a separate bootstrap step that should be documented in the PR description. If they're missing at hook execution time, behavior depends on fallow's flag handling — at best it's a no-op (lint passes without baseline), at worst pre-commit errors out. Check whether these files exist somewhere I missed or need to ship with this PR.

8. Test plan is manual-only.

The PR's test plan is "Tested with 10-element composition covering gsap.to, fromTo, keyframes, stagger, multi-track, locked elements." For a feature this surface-area-heavy (new hotkey, new cursor state, new click handler with three modes — normal/shift/razor — and integration with locked-element rejection, composition source rejection, recording state), there should be at least one integration test exercising the click → useRazorSplit → API → history-entry path. Vitest with msw or a mock fetch would catch the recording-guard regression and the silent-failure regression on every CI run.

Nits

**9. The toolbar selection arrow SVG (<svg width="12" height="12" viewBox="0 0 12 12"><path d="M2 0.5L10 6L6.5 6.5L8.5 11L6.5 11.5L4.5 7L2 9Z"/></svg>) is inlined twice in TimelineToolbar.tsx. Pull into SystemIcons next to Scissors.

**10. setRazorGuideX runs on every mousemove over the timeline. For a feature only active when activeTool === "razor", the no-op move handler still does a setState comparison. Cheap, but you can early-return at the top of the handler.

**11. event.key.toLowerCase() === "b" — the B/V hotkey block runs on every keydown that isn't in an editable target. Consider extracting into a single if (!isEditableTarget(event.target)) parent so the early-return logic isn't duplicated three times.

— Vai

[james-russo-rames-d-jusso]

R2 — both blockers resolved, both concerns still open, plus one R2-new concern.

R1 Blocker A — .fallow/ baselines (RESOLVED) ✅
• lefthook.yml was refactored. The --health-baseline .fallow/health-baseline.json / --dupes-baseline / --dead-code-baseline flags are gone. The new hook is bunx fallow audit --base origin/main --fail-on-issues, no baseline files needed. Pre-commit hook is safe post-merge.

R1 Blocker B — useRazorSplit.ts dropped two guards (RESOLVED) ✅
• (B.1) isRecordingRef guard restored. packages/studio/src/hooks/useRazorSplit.ts:135-138 — if (isRecordingRef?.current) { showToast("Cannot edit timeline while recording", "error"); return; }. Also added to handleRazorSplitAll at line 196. ✅
• (B.2) data.changed check restored. useRazorSplit.ts:96-98 short-circuits executeSplit to changed: false when the server returns { ok: true, changed: false }, and :160-163 surfaces the correct error toast ("Failed to split clip — playhead may be outside the clip") without writing originalContent back to disk. ✅

R1 Concern C — guide-vs-cut x-origin mismatch (STILL OPEN) ⚠️
• packages/studio/src/player/components/Timeline.tsx:367-371 — onMouseMove on the outer container computes setRazorGuideX(e.clientX - rect.left + scrollLeft) (no GUTTER subtraction).
• packages/studio/src/player/components/Timeline.tsx:387-394 — onPointerDown with shift+razor on the scroll container computes x = e.clientX - rect.left + scrollLeft - GUTTER and divides by pps for splitTime.
• The guide line is drawn at line 485 with left: razorGuideX inside the scroll container — so the guide will sit GUTTER pixels to the LEFT of where the actual shift+empty-area cut lands. (The on-clip-click razor path in TimelineCanvas.tsx:368-385 uses clip-local coords, so it's not affected.) Two fixes: either subtract GUTTER from razorGuideX too, or render the guide inside an inner element that starts after the gutter.

R1 Concern D — handleRazorSplitAll sequential N saves (STILL OPEN) ⚠️
• useRazorSplit.ts:207-209 still runs for (const element of splittable) { await handleRazorSplit(element, splitTime); }. Each iteration: HTML mutation + GSAP mutation + saveProjectFilesWithHistory (separate history entry) + reloadPreview (full iframe reload). Vai's R1 #5/#6 on this PR called out the same — the undo problem (N history entries per shift+click) is particularly user-hostile. Either coalesce per-file diffs into one saveProjectFilesWithHistory call, or batch the API into a single /file-mutations/split-many round-trip.

R1 nits
• useRazorSplit.ts:10 — epsilon now extracted to const SPLIT_BOUNDARY_EPSILON_S = 0.03 with a JSDoc comment. Partial — TimelineCanvas.tsx:372 still has a local const epsilon = 0.03; (unchanged). Worth importing the constant for consistency, since these two callsites need to agree.
• useRazorSplit.test.ts — still missing. PR adds ~215 LOC of hook logic with zero automated tests. Recording-guard regressions, silent-failure regressions, and the GSAP-step-failure path below would all be CI-catchable. (concern-leaning nit, not blocker)

R2-new concern
• Silent GSAP-step failure on a split-applied element. useRazorSplit.ts:51-78 splitGsapAnimations returns null if the GSAP endpoint returns !response.ok OR data.ok === false. executeSplit:103-114 then proceeds with if (gsapContent) patchedContent = gsapContent; — meaning the HTML split is applied but animations still target the original domId. The disk write goes through, changed: true, success toast is shown — but the clip plays wrong because animations target the pre-split element. Either bail with an error toast when GSAP step fails (and don't write the HTML half), or surface a degraded-success toast ("Split clip but animation retarget failed"). Currently silent.

What I didn't re-verify (Vai's lane)
• Click-to-split position correctness against Adobe Premiere parity.
• Frame-snap behavior at 30fps/60fps with the 0.03s epsilon.
• Whether the reloadPreview() inside useRazorSplit.ts:176 is sufficient for the runtime to pick up split DOM + split GSAP tweens on the next play.

Acknowledging Vai's R1 finding addressed at R2
• Vai's R1 #1 (reloadPreview regression on move/resize) is fully resolved — useTimelineEditing.ts:119-173 no longer calls reloadPreview in handleTimelineElementMove or handleTimelineElementResize. Iframe-flash on every drag-end is gone. ✅

— Rames D Jusso

[vanceingalls]

R2 — blocking

Re-checked the open items from R1 + Rames's R2 pass. The formal R1 blockers (.fallow/ config, isRecordingRef guard, data.changed check, reloadPreview regression) are clean. Two of Rames's three open concerns are real; one I disagree with.

Blockers

1. Silent partial failure when splitGsapAnimations fails (Rames's R2-new) — confirmed

useRazorSplit.ts:51-78:

async function splitGsapAnimations(...): Promise<string | null> {
  const response = await fetch(`/api/projects/${projectId}/gsap-mutations/...`, ...);
  if (!response.ok) return null;
  const data = (await response.json()) as { ok?: boolean; after?: string };
  return data.ok && data.after ? data.after : null;
}

useRazorSplit.ts:103-114:

if (element.domId) {
  const gsapContent = await splitGsapAnimations(...);
  if (gsapContent) patchedContent = gsapContent;
}

When the GSAP-mutation call fails (transient 500, network timeout, parser failure on the server) splitGsapAnimations returns null, patchedContent stays as the HTML-only split result, the file write proceeds, and the success toast fires (Split ${label} at ${time}s).

User-visible effect: the second-half clone exists in the HTML with a new id, but every GSAP tween still targets the original domId. The first half plays its animations; the second half is unanimated. If those animations set initial state (opacity: 0 at t=0), the second half may render invisible forever, with no error indication.

This needs to either:

• distinguish "no GSAP block to split" (benign — server returns 400 with error: "no GSAP script found in file" at files.ts:1029-1031) from transient errors (must surface),
• OR fail the whole split atomically: if HTML split succeeded but GSAP split failed, undo the HTML mutation before returning, and surface an error toast.

Either approach is fine; silent partial-success is not.

2. Sequential handleRazorSplitAll creates N undo steps and races on file contents (Rames C-D) — confirmed

useRazorSplit.ts:204-209:

const splittable = elements.filter(...);
for (const element of splittable) {
  await handleRazorSplit(element, splitTime);
}

Each handleRazorSplit call independently calls saveProjectFilesWithHistory, so a 6-clip "split all" produces 6 separate history entries. Cmd-Z undoes one clip's split at a time. UX-wise this is wrong — the user did one operation; one undo should reverse it. More importantly, each iteration reads file content fresh (readFileContent inside executeSplit), so the iterations are interdependent (each must see the previous write). If any iteration's read precedes the previous iteration's write (e.g., the write API returns success before the read API sees the new content — possible with caching, retries, eventual consistency in storage backends), the second iteration corrupts the file by writing a stale-base patch.

Fix: collect all mutations, write once, record one history entry. The split-element + split-animations server endpoints are independent of each other across clips, so they can be batched: collect (targetPath, mutation) pairs for all splittable elements, apply sequentially in-memory, write each unique file once with a single history entry.

Not blocking — visual nit / sanity check

3. Guide-vs-cut x-origin (Rames's Concern C) — I don't see a mismatch.

I traced the math; both the guide and the cut land at the same content-x:

• Guide (Timeline.tsx:367-371): razorGuideX = e.clientX - rect.left + scrollLeft. Rendered with left: razorGuideX inside the timeline's relative-positioned container.
• Cut (Timeline.tsx:387-391): splitTime = (e.clientX - rect.left + scrollLeft - GUTTER) / pps. The cut visually lands at GUTTER + splitTime * pps = razorGuideX.

Both resolve to the same x-position at the cursor. The GUTTER subtraction in the cut isn't a visual offset, it converts from content-x to seconds. If there's a visible mismatch in practice, it's not from the formulas as written. Worth a quick manual verification when you spin the UI up — but I don't think this needs a code change.

Other

• lefthook.yml largefiles hook addition is reasonable and orthogonal to the razor work. Self-contained, doesn't change PR semantics. Fine.
• The B / V / Escape hotkeys (useAppHotkeys.ts:340-385) bypass isEditableTarget correctly. Good.
• activeTool === "razor" early-return in TimelineCanvas.tsx:312 prevents shift-click range selection from interfering with razor-shift-click split-all. Good.
• canSplitElement is now consolidated (good — same predicate fires in Toolbar, hotkeys, and useRazorSplit).

Summary

Two blockers — silent partial failure and undo-grain/race. Concern C I disagree with. Once those land I'll re-stamp.

Review by Vai

[james-russo-rames-d-jusso]

R3 — Concerns C + D resolved, R2-new silent-fail partial (5xx hard-errors, 4xx still silent), nits mostly resolved.

HEAD moved 95210798 → 7ad1dd12. Delta is +547/-208 across 12 files (rewritten useRazorSplit.ts +289 new, Timeline.tsx/TimelineCanvas.tsx reworked, new largefiles lefthook hook, useAppHotkeys.ts + TimelineToolbar.tsx widened via canSplitElement).

R2-Concern C (guide-vs-cut x-origin mismatch) — RESOLVED, and I was wrong ❌→✅

• Vai disagreed with my R2 take and was correct. Re-tracing on this HEAD: guide at Timeline.tsx:367-371 is rendered with left: razorGuideX INSIDE scrollRef (which already sits after the gutter). Cut at :387-394 subtracts GUTTER not as a visual offset but as the content-x → seconds conversion (splitTime = x / pps). Both resolve to the same screen-x at the cursor. My R2 finding was a false positive — I was conflating "the cut math subtracts GUTTER" with "the guide should subtract GUTTER too." Vai's R2 trace was right. No code change needed; correctness vindicated by re-reading carefully.

R2-Concern D (sequential split-all + N undo steps + race) — RESOLVED ✅

• useRazorSplit.ts:194-256 handleRazorSplitAll rewritten. New flow:
  1. Pre-fetch originals Map<path, content> via single read per unique file (:217-222).
  2. Loop executeSplit per element, collect finalContent Map (:226-232).
  3. Write each unique file once (writeProjectFile inside loop OK since each path is independent; could be batched further but acceptable).
  4. ONE recordEdit call with all { before, after } per file (:240-249).
  5. ONE reloadPreview() after the loop (:251).
• Result: shift+click on N clips = 1 history entry (single Cmd-Z to undo), N file writes (one per unique source file), 1 preview reload. Solves both Vai R1 #5/#6 and my R2 D. The race window from R2 is also closed because each iteration's executeSplit re-reads the current disk content via readFileContent — and the loop is serialized via await, so iteration N+1 sees iteration N's write. The originals map captures pre-split state for history.

R2-new (silent partial failure on GSAP fail) — PARTIAL ⚠️

• useRazorSplit.ts:51-90 splitGsapAnimations rewritten. Now:
  • 5xx server errors → throw new Error("GSAP animation split failed (server error)") (:77). Caller's outer try/catch surfaces error toast and does NOT write the HTML-only patch. ✅ for the 5xx case.
  • Non-OK <500 (e.g. 400 "no GSAP script found in file" from files.ts:1029-1031) → returns { content: null }. executeSplit:130 then keeps HTML-only patchedContent, write proceeds, success toast fires. The 400 path is legitimate "no GSAP to split" (benign), but a 400 from a parser failure (genuinely bad) would silently degrade. The route's only documented 400 is the no-GSAP case, so this is probably fine in practice — but the code doesn't distinguish them. ⚠️
  • OK responses with data.ok === false → content: null. Similar silent degradation if the engine bailed for non-route-validation reasons.
  • NEW: skippedSelectors is now surfaced via a separate info toast at :181-186 ("Some animations use non-ID selectors (...) and were not retargeted"). This is the right shape for partial-success surfacing — applies the pattern; just not extended to the GSAP-step-failed case.
• Verdict: 5xx-path correct, 4xx-path partial. The exact case I described (HTML split succeeds, GSAP step fails on a clip that has animations, silent success) is still possible on a 4xx, but the route's actual 4xx codepath is narrow. Not blocking but worth a follow-up.

R2 nits

• Epsilon constant duplication: TimelineCanvas.tsx now imports SPLIT_BOUNDARY_EPSILON_S from ../../utils/timelineElementSplit (:21) and uses it at the click handler (:372-378). Local const epsilon = 0.03 is gone. ✅
• No useRazorSplit.test.ts: gh api .../contents/packages/studio/src/hooks/useRazorSplit.test.ts → 404. Still not present. The refactored hook is now meaningfully more complex (290 LOC, multi-file batching, error paths, recording guard, partial-success toast surfacing). Strong CI candidates: recording-guard regression, GSAP-fail-4xx silent path, multi-clip split-all atomicity, originals map correctness. ⚠️ concern-leaning nit.

R3-new findings

• None blocking. Two minor observations:
  • useRazorSplit.ts:262-271 recordEdit for handleRazorSplitAll uses Object.fromEntries([...finalContent].map(...)) to build the files record, then for (const [path, content] of finalContent) files[path] = content; also builds an unused files: Record<string, string> (lines 244-245). Dead variable — files is built and never read; the recordEdit call uses its own Object.fromEntries(...). Cleanup nit.
  • useRazorSplit.ts:251 calls reloadPreview() after the batch — but the batch already includes a await recordEdit(...) which itself may trigger downstream reactivity. Not a regression vs R2; just noting that recordEdit and reloadPreview could potentially be coalesced if recordEdit is documented to NOT trigger a player refresh.

Lefthook delta (R3-new, not blocking)

• lefthook.yml:26-32 adds a largefiles pre-commit hook running ./scripts/check-large-files.sh. Reasonable, scoped, doesn't affect razor logic. Self-contained.

What I didn't re-verify (Vai's lane)

• Click-to-split position correctness against Adobe Premiere parity.
• Frame-snap behavior at 30fps/60fps with the 0.03s epsilon.
• Whether reloadPreview() after the split-all is sufficient for the runtime to pick up N split DOM + N split GSAP tweens on the next play.
• Whether the data.changed === false benign no-op should still suppress the success toast (:171-174 — currently returns silently, no toast).

Vai's R1/R2 findings status on this PR (acknowledging her lane)

• Vai R1 #1 (reloadPreview regression on move/resize) — resolved at R2, holds at R3. ✅
• Vai R1 #2 (recording guard) — resolved at R2, holds. ✅
• Vai R1 #3 (silent failure toast) — partially resolved; canSplitElement no-op at :151 and boundary-clamp at :155-160 still return silently, but the recording-guard and the GSAP-skipped-selectors paths now toast. ⚠️ remaining UX gaps are minor.
• Vai R1 #5/R2 #2 (sequential split-all + N undo steps) — same as my R2 D, RESOLVED. ✅
• Vai R2 #1 (silent partial failure on GSAP fail) — same as my R2-new, PARTIAL. ⚠️

— Rames D Jusso

[vanceingalls]

R3 — REQUEST_CHANGES

Still blocking: 4xx silent partial failure [from R2, partially addressed]

The server-side split-element route writes HTML to disk immediately (line 855 writeFileSync). GSAP mutation writes in a second call (line 1066). These are two separate server writes with no transaction.

Current state after R3:

• 5xx path: throws → outer catch fires error toast. But HTML is already on disk with the cloned element and no animations retargeted. The rollback doesn't happen — the file stays corrupted.
• 4xx path: returns { ok: false } / content: null → falls through silently → success toast fires. File on disk has the HTML split but no GSAP retargeting. User sees success, composition is broken.

The 5xx error toast is an improvement, but both paths leave the file in a corrupt state. The fix needs to either:

1. Roll back the HTML split on GSAP failure (restore original file from a snapshot taken before split-element was called), OR
2. Make the split-element route transactional (run HTML + GSAP in a single server op that either fully commits or fully rolls back)

The 4xx path also needs a hard-error toast, not silent success.

R2 items resolved ✅

• Guide-vs-cut x-origin — Rames acknowledged his R2 concern was wrong. The GUTTER subtraction in cut-time math aligns with the scroll content offset. Consistent. ✅
• Sequential split-all — Rewritten: pre-fetch originals, one recordEdit for all clips, single undo entry. ✅
• Epsilon constant — Now imported from timelineElementSplit. ✅

Nits

• files Record at line 244-245 — built but unused (recordEdit uses its own Object.fromEntries). Dead variable, drop it.
• useRazorSplit.test.ts — still missing; the hooks are not tested.

---

Review by Vai

[vanceingalls]

R3 — APPROVE conditional on #1330 landing. R2 blockers on the razor UI itself are resolved; the remaining open issues are downstream of #1330's engine correctness, not this PR's responsibility.

Re-traced against HEAD 7ad1dd12. The UI/orchestration layer is in good shape; the things that were R2 blockers on this PR are fixed. Approving so this can land alongside #1330 once that one's correctness blockers are addressed.

Resolved at R3

• Silent partial failure on 5xx (R2 blocker 1, partial): useRazorSplit.ts:51-90. splitGsapAnimations now throws on response.status >= 500, which the outer try/catch at :524-527 surfaces as an error toast. The HTML split has already been written server-side at this point (files.ts:855), so the on-disk state is HTML-cloned-but-not-animation-retargeted — but the user is at least told it failed and can undo. ✅ for the 5xx path.
• Sequential split-all batched into one history entry (R2 blocker 2): useRazorSplit.ts:540-610. handleRazorSplitAll now: (a) pre-fetches originals Map per unique path (:557-563), (b) loops executeSplit per element collecting results into finalContent, (c) calls recordEdit once with all { before, after } per file (:583-592), (d) calls reloadPreview() once after the loop. Single Cmd-Z undoes the whole batch. ✅
• Race condition in sequential split-all: each iteration re-reads disk content via readFileContent, and await serializes the loop, so iteration N+1 sees iteration N's write. ✅
• Guide-vs-cut x-origin (R2 concern C from Rames): re-traced, this was a false positive at R2. Guide at Timeline.tsx:864-868 is rendered with left: razorGuideX (raw scroll-container-relative x); the absolute-positioned guide sits inside the scroll container, which has its content origin at GUTTER. The cut math at :879-887 subtracts GUTTER to convert to seconds. Both resolve to the same screen pixel at the cursor — no visual mismatch. ✅

Important but not blocking — 4xx silent partial failure path still open.

useRazorSplit.ts:401-413. Non-response.ok responses with status < 500 return { content: null } and the caller treats this as "GSAP step had nothing to retarget, proceed with HTML-only." In practice the only documented 400 from files.ts:1029-1031 is the no-GSAP-script-found case, which is genuinely benign. But the code doesn't distinguish a legitimate no-op from a parser-validation failure — both fall through silently with a success toast.

The HTML split has already been persisted to disk by the time we reach this branch (files.ts:855 writes synchronously). If the GSAP step legitimately failed on a clip that had animations, the user gets a success toast and a clip whose clone has no animation retargeting. Lower-frequency than 5xx, but the same shape of bug.

Suggest distinguishing in the response body: { ok: false, reason: "no-gsap" | "parser-failed" | ... } and surfacing the latter as an error. Not blocking — narrow window in practice.

Important but not blocking — no test coverage for useRazorSplit.ts.

packages/studio/src/hooks/useRazorSplit.test.ts does not exist (verified via gh api .../contents/... returning 404). The hook is now 290 LOC with: recording guard, boundary epsilon clamping, per-clip executeSplit error paths, split-all atomicity, partial-success toast surfacing, originals map race-safety. Strong CI candidates:

1. Recording-guard regression (:478-482, :542-546) — if isRecordingRef.current === true, no mutation happens, error toast fires.
2. Boundary clamp (:486-494) — splits within SPLIT_BOUNDARY_EPSILON_S of clip edges are no-ops.
3. GSAP-5xx-throws-and-aborts-write (:401-403) — saveProjectFilesWithHistory is NOT called when GSAP throws.
4. GSAP-4xx-returns-null-but-write-proceeds (:401-413 then :442-443) — current behavior; lock in as expected or fix it.
5. Split-all atomicity (:540-610) — N clips → exactly 1 recordEdit call, exactly 1 reloadPreview(), originals map captures pre-split state.
6. skippedSelectors toast surfaces (:518-523) — bypassed in split-all path, which silently drops the warning. Worth verifying this is intentional.

Given the engine in #1330 has known correctness gaps, end-to-end tests against the hook would catch regressions when the engine ships fixes. Not blocking this PR, but a meaningful gap.

Minor nits

• useRazorSplit.ts:580-581 — files: Record<string, string> is built into files but the variable is never read; recordEdit builds its own files via Object.fromEntries(...) on line 586. Dead code; can be removed.
• useRazorSplit.ts:518-523 — skippedSelectors toast tone is "info" but the user-visible impact (animation now fires on both clip halves for class-selector tweens, per #1330 analysis) is "the split partially failed." Either tone should be "error" or the message should be more explicit about the consequence. Trivial copy change.
• Timeline.tsx:885 — shift+click at scroll-container-x < GUTTER (i.e., in the gutter area) is clamped to splitTime = 0. Edge case but worth a quick verify that this doesn't try to split the first clip at its start boundary (which canSplitElement + epsilon clamp should already reject downstream — confirmed at useRazorSplit.ts:489-494).
• useRazorSplit.ts:594 — reloadPreview() is called after recordEdit completes. If recordEdit triggers downstream reactivity that also reloads, this is a redundant call. Not a bug, just a potential coalescing opportunity. Defer.

Resolved nits (R2)

• SPLIT_BOUNDARY_EPSILON_S consolidated into timelineElementSplit.ts and imported across consumers (Timeline + TimelineCanvas + useRazorSplit). ✅
• Local epsilon constants removed.

Architectural callout (not a blocker for this PR)

This PR ships a UI that calls into #1330's engine. If #1330 lands with its current correctness bugs (spanning .to() 2x-speed, spanning keyframe wrong-timing — see my review on that PR), then the razor tool will produce visibly-broken playback for the most common cases. Users will get success toasts and broken animations. Recommend gating the toolbar button behind a feature flag until #1330's blockers are addressed — or at minimum, downgrading the success toast to a warning until then. Vance's call.

Verdict

Approving. This PR's own R2 blockers are resolved or have a sound resolution path. The remaining issues are downstream (engine correctness — #1330) or non-blocking (4xx silent path, test coverage, minor nits). Merge order should be: fix #1330, land #1329 (already approved) + #1330, then land this.

— Vai

[james-russo-rames-d-jusso]

R4 — three of four R3 carry-overs RESOLVED. Feature-flag gating verified end-to-end. One R3 nit (missing tests) still open. LGTM on canonical axes; conditional on #1330 engine fixes per Vai.

HEAD moved 7ad1dd12 → c66bce20. Blob-SHA check confirms useRazorSplit.ts content changed (a6a44e76 → 4c8daefd). New file in this delta: packages/studio/src/components/editor/manualEditingAvailability.test.ts (test for the env-flag resolver). Stack base updated to 072d9479 (#1330's rebased HEAD). Stack chain intact: 1331 → 1330 → 1329 → main. ✅

R3 carry-over verdicts

R3 #1 — 4xx silent-success-toast path (RESOLVED) ✅

• useRazorSplit.ts:71-83 splitGsapAnimations now distinguishes 4xx codepaths:
  • The specific benign case (errorBody.error === "no GSAP script found in file") returns { content: null } without throwing — legitimate no-op.
  • All other non-OK responses (4xx with any other error message, plus 5xx) now throw new Error(errorBody?.error ?? "GSAP animation split failed (...)").
• executeSplit:130-137 catches the throw and rolls back the HTML half via await writeProjectFile(targetPath, originalContent) before re-throwing. The outer try/catch in handleRazorSplit surfaces the error toast. No silent partial-success disk corruption on any error path. This is the rollback shape Vai R3 explicitly asked for.

R3 #2 — epsilon constant nit (HOLDS) ✅

• SPLIT_BOUNDARY_EPSILON_S is imported from ../utils/timelineElementSplit in useRazorSplit.ts:10 and used at :159-161 for boundary clamping. No re-drift to a local constant.

R3 #3 — missing useRazorSplit.test.ts (NOT RESOLVED) ❌

• gh api .../contents/packages/studio/src/hooks/useRazorSplit.test.ts?ref=c66bce20 returns 404. The hook is now 303 LOC with: recording guard, boundary epsilon clamping, GSAP-throw rollback, partial-success skippedSelectors toast, multi-clip split-all atomicity with the originals-map race-safety. Strong CI candidates that would have caught the R3 carry-overs themselves:
  1. GSAP-step-5xx → outer catch fires error toast, file rolled back.
  2. GSAP-step-4xx-other → same rollback path.
  3. GSAP-step-4xx-with "no GSAP script found in file" → benign no-op, HTML-only split succeeds.
  4. Recording-guard → handler is a no-op + error toast.
  5. Split-all atomicity → N clips, exactly one recordEdit, exactly one reloadPreview, originals map captures pre-split state per unique file.
• Still concern-leaning nit, not blocking on its own.

R3 #4 — dead files Record variable (RESOLVED) ✅

• handleRazorSplitAll rewritten. No standalone files local. The recordEdit call at :240-249 builds the files record inline via Object.fromEntries([...finalContent].map(...)). Clean.

R4 NEW — feature flag verification (VITE_STUDIO_ENABLE_RAZOR_TOOL)

This is the meaningful new surface this round. The whole tool is now behind a build-time + runtime flag. Verified end-to-end:

Flag mechanism (manualEditingAvailability.ts:81-85)

• STUDIO_RAZOR_TOOL_ENABLED = resolveStudioBooleanEnvFlag(env, ["VITE_STUDIO_ENABLE_RAZOR_TOOL", "VITE_STUDIO_RAZOR_TOOL_ENABLED"], false).
• Default = false — flag-off is the prod-safe default. ✅
• Two flag-name aliases supported, matches the pattern used by sibling flags (STUDIO_PREVIEW_MANUAL_EDITING_ENABLED, STUDIO_INSPECTOR_PANELS_ENABLED, etc.).
• The resolver at manualEditingAvailability.ts:7-26 uses an explicit TRUTHY/FALSY allowlist ("1","true","yes","on","enabled" vs "0","false","no","off","disabled"), NOT Boolean(value). So the classic Boolean("false") === true footgun is avoided. ✅
• Build-time import.meta.env is merged with runtime window.__HF_STUDIO_ENV__ (injected by the embedded Hono server from the user's shell env). Runtime overrides take precedence — operator can flip the flag in prod without a rebuild. Smart.

Gating completeness — verified ALL entry points

• Toolbar buttons: TimelineToolbar.tsx:117 — entire {STUDIO_RAZOR_TOOL_ENABLED && (...)} block (selection-arrow + scissors) only renders when flag on. ✅
• B-key hotkey: useAppHotkeys.ts:272 — STUDIO_RAZOR_TOOL_ENABLED && event.key.toLowerCase() === "b" short-circuits. Flag off → B-key is dead. ✅
• Shift-click split-all entry (Timeline.tsx:388): gated by activeTool === "razor". activeTool can ONLY become "razor" via the toolbar button or the B-key, both of which are flag-gated. Transitively safe. ✅
• Crosshair cursor + razor-guide line (Timeline.tsx:366-372, :485-494): gated by activeTool === "razor". Same transitive guarantee. ✅
• On-clip razor click (TimelineCanvas.tsx): gated by activeTool === "razor". Same. ✅
• V-key and Escape (useAppHotkeys.ts:287, :301): NOT explicitly flag-gated, but their effect (setActiveTool("select")) is a no-op when flag off (default is already select). Harmless. ✅

Default-value safety

• gh api .../git/trees/c66bce20?recursive=true | grep env shows only .env.example exists at the repo root. Read .env.example content (base64-decoded) — no VITE_STUDIO_ENABLE_RAZOR_TOOL=true leak. ✅
• No .env / .env.local / .env.production committed. ✅

Test coverage of the flag

• manualEditingAvailability.test.ts exists and exercises resolveStudioBooleanEnvFlag with: missing env, empty string, unknown values, explicit truthy/falsy strings, legacy-alias precedence, preferred-flag-overrides-alias. Coverage is on the resolver, not directly on STUDIO_RAZOR_TOOL_ENABLED — but since the resolver is the only path, and the razor flag uses the exact same shape as the other tested flags, this is sufficient. ✅

Tear-down / rollback shape

• When the flag rolls out for removal: a single grep for STUDIO_RAZOR_TOOL_ENABLED covers four call sites (TimelineToolbar.tsx, useAppHotkeys.ts, manualEditingAvailability.ts). Plus the env-name strings if you want to clean the resolver call too. Localized, easy. ✅

Type-safety nit (R4-new, not blocking)

• No vite-env.d.ts exists in the repo (verified via tree walk). import.meta.env.VITE_STUDIO_ENABLE_RAZOR_TOOL is typed as any at the call site in manualEditingAvailability.ts:43. The resolver mitigates this with typeof value === "boolean" / typeof value !== "string" runtime guards, so a typo'd env name silently falls back to the declared default — which for razor is false (safe). Adding a typed ImportMetaEnv interface would catch typos at compile time but isn't load-bearing. Nit only.

R4-NEW findings beyond the flag

Spot-checked the rest of the diff against my R3 review:

• Timeline.tsx:367-371 — setRazorGuideX on onMouseMove is now gated by activeTool === "razor" at the FIRST check inside the handler, so flag-off path doesn't even compute the rect (matches Vai's R2 nit-10 suggestion). ✅
• useRazorSplit.ts:217-249 — handleRazorSplitAll atomicity preserved across the rebase: pre-fetch originals map (:217-222), in-loop executeSplit collect (:226-232), single recordEdit with all files (:240-249), single reloadPreview (:251). ✅
• handleRazorSplitAll does NOT surface skippedSelectors toasts from per-clip results — the per-clip executeSplit returns skippedSelectors but the multi-clip path drops them. Minor: a user shift-clicking N clips with class-selector animations gets no info toast. Not blocking; can be a follow-up.

Vai's lane (not weighing)

• Click-to-split position vs Adobe Premiere parity.
• Frame-snap behavior at 30/60fps with the 0.03s epsilon.
• Whether the upstream #1330 engine math (Vai's R3 B1/B2 blockers — spanning .to() 2× speed, spanning-keyframe retarget-without-reposition) is fixed at #1330's HEAD 072d9479. Until that's clear, the UI here ships into a "calls into a known-incorrect engine" state.

Verdict

• Canonical axes clean: feature flag is correctly defaulted off, gates every entry point, has tests via the resolver, no accidental .env leak.
• Three of four R3 carry-overs resolved. Test-file nit (R3 #3) still open but not load-bearing on its own.
• Merge order should still be: #1329 lands, then #1330 once Vai's engine math blockers are addressed, then this one alongside.

— Rames D Jusso

[vanceingalls]

R4 — APPROVE at c66bce20

#1331 is the UI layer on top of #1330's engine. The core blockers are in #1330; this PR's job is to plumb the feature and gate it behind a flag. It does both cleanly.

---

ENV flag ✅

manualEditingAvailability.ts adds STUDIO_RAZOR_TOOL_ENABLED with default false. Both the toolbar buttons and the TimelineCanvas click handler are guarded behind this flag. The activeTool store state resets to "select" on composition reset.

---

Wiring ✅

useRazorSplit is a clean extraction: executeSplit handles the full HTML+GSAP mutation sequence, single-split has proper rollback (restores original content on GSAP failure after HTML write), and handleRazorSplit gates on canSplitElement + boundary epsilon before calling through.

Hotkeys B (toggle razor), V (select), and Escape (exit razor) are guarded behind STUDIO_RAZOR_TOOL_ENABLED for the razor toggle (only V and Escape always register — intentional since those are no-ops in select mode). ✅

---

Important: handleRazorSplitAll has no rollback on partial failure

useRazorSplit.ts ~line 583–591:

for (const element of splittable) {
  const result = await executeSplit(...);
  if (result.changed) {
    finalContent.set(result.targetPath, result.patchedContent);
    await writeProjectFile(result.targetPath, result.patchedContent);  // written to disk immediately
    splitCount++;
  }
}

If split #3 of 5 throws, elements 1 and 2 are already persisted. The catch at the end only shows a toast — no restoration. Contrast with single-split executeSplit which does restore on GSAP failure. Behind VITE_STUDIO_ENABLE_RAZOR_TOOL=false so not a deploy concern right now, but this needs a rollback pass (collect all results first, then write atomically) before the flag goes prod.

---

— Vai

The base branch was changed.

[github-actions]

Fallow audit report

Found 17 findings.

Details

Severity	Rule	Location	Description
minor	fallow/high-crap-score	packages/studio/src/components/StudioPreviewArea.tsx:125	'<arrow>' has CRAP score 30.0 (threshold: 30.0, cyclomatic 5)
minor	fallow/high-crap-score	packages/studio/src/components/StudioPreviewArea.tsx:180	'<arrow>' has CRAP score 42.0 (threshold: 30.0, cyclomatic 6)
major	fallow/high-crap-score	packages/studio/src/components/TimelineToolbar.tsx:51	'useKeyframeToggle' has CRAP score 56.0 (threshold: 30.0, cyclomatic 7)
major	fallow/high-crap-score	packages/studio/src/components/TimelineToolbar.tsx:66	'computePct' has CRAP score 72.0 (threshold: 30.0, cyclomatic 8)
minor	fallow/high-crap-score	packages/studio/src/components/TimelineToolbar.tsx:176	'<arrow>' has CRAP score 42.0 (threshold: 30.0, cyclomatic 6)
minor	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:148	'handleUndo' has CRAP score 42.0 (threshold: 30.0, cyclomatic 6)
minor	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:173	'handleRedo' has CRAP score 42.0 (threshold: 30.0, cyclomatic 6)
critical	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:227	'<arrow>' has CRAP score 6162.0 (threshold: 30.0, cyclomatic 78)
minor	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:465	'syncPreviewTimelineHotkey' has CRAP score 30.0 (threshold: 30.0, cyclomatic 5)
major	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:512	'syncPreviewHistoryHotkey' has CRAP score 72.0 (threshold: 30.0, cyclomatic 8)
major	fallow/high-crap-score	packages/studio/src/hooks/useRazorSplit.ts:53	'splitGsapAnimations' has CRAP score 72.0 (threshold: 30.0, cyclomatic 8)
critical	fallow/high-crap-score	packages/studio/src/hooks/useRazorSplit.ts:229	'handleRazorSplitAll' has CRAP score 210.0 (threshold: 30.0, cyclomatic 14)
critical	fallow/high-crap-score	packages/studio/src/hooks/useTimelineEditing.ts:176	'handleTimelineElementDelete' has CRAP score 132.0 (threshold: 30.0, cyclomatic 11)
critical	fallow/high-crap-score	packages/studio/src/hooks/useTimelineEditing.ts:250	'handleTimelineAssetDrop' has CRAP score 156.0 (threshold: 30.0, cyclomatic 12)
critical	fallow/high-crap-score	packages/studio/src/hooks/useTimelineEditing.ts:332	'handleTimelineFileDrop' has CRAP score 110.0 (threshold: 30.0, cyclomatic 10)
minor	fallow/high-crap-score	packages/studio/src/player/components/TimelineCanvas.tsx:262	'<arrow>' has CRAP score 37.1 (threshold: 30.0, cyclomatic 11)
major	fallow/high-crap-score	packages/studio/src/player/components/TimelineCanvas.tsx:312	'<arrow>' has CRAP score 63.6 (threshold: 30.0, cyclomatic 15)

Generated by fallow.