feat(studio): app shell swap — mount EditorShell, retire the legacy engine#2213
feat(studio): app shell swap — mount EditorShell, retire the legacy engine#2213ukimsanov wants to merge 1 commit into
Conversation
2b93f60 to
5c41357
Compare
08ca624 to
f07ff86
Compare
5c41357 to
6f9c592
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). This is one of the critical swap PRs — reviewed with extra scrutiny. No blockers. — Miga
vanceingalls
left a comment
There was a problem hiding this comment.
R1 — Batch E · app-shell swap + legacy engine retirement
Reviewed as PR 5 of 6 in Batch E — the highest-blast-radius PR in the 25-PR NLE stack. Head SHA 6f9c592917d2dbc5efb6660066b6b793bc50d25b. -5,796 deletions across 44 files, including the .fallowrc.jsonc TEMP(studio-dnd) block, both legacy shells, all group-editing helpers, and a small cross-package trim into packages/core/.
Verdict: 🟢 LGTM
The cutover collapses cleanly. Zero dangling imports into deleted modules. No cross-package consumers of the removed core symbols. All TEMP(studio-dnd) fallow entries — added across the whole stack — are removed here. EditorShell mounts at the swap site with all callbacks wired.
Deletion audit — 8 removed source files
At tip ce89d8e (Batch E merged-tree), grepped from "..." imports across packages/ for each truly-removed module:
| Removed module | Dangling imports |
|---|---|
StudioPreviewArea (-500) |
0 |
NLELayout (-591) |
0 |
useTimelineGroupEditing (-370) |
0 |
timelineLayerDrag (-383) |
0 |
useTimelineEditing.test.tsx (-963) |
0 |
timelineLayerDrag.test.ts (-200) |
0 |
NLELayout.test.ts (-12) |
0 |
timelineDiscovery.test.ts (-90) |
0 |
Every deleted source has its test deleted alongside it. timelineDiscovery.ts is trimmed (-17) rather than removed — only isEditableTarget remains; its test file was deleted since the surface it covered is gone. That's an intentional shrink, not an orphan.
.fallowrc.jsonc — TEMP(studio-dnd) sweep
Net -278 / +1 (the +1 is a comment rewrite for the TimelineEditorNotice.tsx entry). 57 path entries removed from the TEMP(studio-dnd) block. Enumerated for Phase-3 cross-check against Batches C/D additions:
EditorShell.tsx,StudioPreviewArea.tsx,CanvasContextMenu.tsx,DomEditOverlay.tsx,DomEditSelectionChrome.tsx,LayersPanel.test.ts,canvasContextMenuZOrder.{ts,test.ts},domEditNudge.ts,domEditOverlayGeometry.ts,domEditOverlayGestures.ts,manualOffsetDrag.ts,useDomEditNudge.tsAssetPreviewOverlay.tsx,NLEContext.{tsx,test.ts},NLELayout.{tsx,test.ts},PreviewOverlays.tsx,PreviewPane.tsx,TimelinePane.tsx,useTimelineEditCallbacks.tsAssetCard.tsx,AssetContextMenu.tsx,assetHelpers.tsDomEditContext.tsx,StudioContext.tsxtimelineEditingHelpers.ts,timelineElementsMove.ts,useDomEditWiring.ts,useGsapSelectionHandlers.ts,useTimelineEditing.ts,useTimelineEditingDrops.tsTimeline.tsx,TimelineCanvas.tsx,TimelineLanes.tsx,TimelineOverlays.tsx,timelineClipChildren.tsx,timelineClipDragPreview.ts,timelineClipDragTypes.ts,timelineCollision.test.ts,timelineEditing.ts,timelineLayout.ts,timelineStackingSync.test.tsuseTimelineClipDrag.ts,useTimelineEditPinning.ts,useTimelineGeometry.ts,useTimelineMarqueeSelection.ts,useTimelineStackingSync.tsuseTimelinePlayer.ts,useTimelinePlayerLoop.ts,useTimelineSyncCallbacks.tstimelineElementHelpers.ts,timelineIframeHelpers.tsassetPreviewStore.ts,studioHelpers.ts,timelineAssetDrop.ts
Total: 57 distinct paths removed. Phase-3 aggregate will diff against Batches C/D's additions.
Cross-package concern — packages/core/src/runtime/timeline.ts (+6/-59)
Removed: file-local helper normalizeTrackAssignments and its lookup table KIND_ORDER (both const/function, not exported). Behavior change: collectRuntimeTimelinePayload no longer splits mixed-kind tracks onto separate row indices — it honors data-track-index verbatim.
Blast radius check:
- Grepped
normalizeTrackAssignmentsandKIND_ORDERacross allpackages/**/*.{ts,tsx}at tip → 0 external references. Both symbols were private to this file. runtime/timelineis NOT exported from the@hyperframes/corebarrel (packages/core/src/index.ts— no runtime/timeline re-export). External surface untouched.- Internal caller:
packages/core/src/runtime/init.ts:1884callscollectRuntimeTimelinePayloadand posts the result ashf-previewpayload / stores atwindow.__clipManifest. Consumers ofclip.trackdownstream:timeline.tsitself derivesgsapTrack = max(clips.map(c => c.track)) + 1andoverlayTrackthe same way for elements without an authoreddata-track-index. Under the removed normalizer these were derived over contiguous indices; now they're derived over authored indices. In both regimes GSAP/overlay clips still sort strictly above authored clips (it'smax + 1), just at higher absolute values. No functional break.timeline.test.tshas a singletrackassertion (expect(result.clips[0].track).toBe(0)) withdata-track-index="0"— still holds under authored-track semantics.
The PR comment justifies this as the correct NLE behavior ("Track index is display-only; render never reads it"). Verified — RuntimeTimelineClip.track isn't consumed for rendering, only for display sort order.
App-shell swap verification
App.tsx (+89/-116) replaces StudioPreviewArea with EditorShell at the same slot inside viewModeValue.viewMode !== "storyboard" gate. All drag/drop/gesture/toolbar callbacks are threaded through (handleTimelineFileDrop, handleTimelineAssetDrop, handleTimelineBlockDrop, handleTimelineElementMove/Resize/Delete/Split, handleRazorSplit(All), handleToggleTrackHidden, renderClipContent, timelineToolbar, blockPreview, gestureOverlay). Left/right slots (StudioLeftSidebar, StudioRightPanel) are passed as slot props.
New: handleTimelineElementsMove (from Batch D multi-select) is now wired at the shell — good. Removed: handleTimelineGroupMove/GroupResize — the group-editing surface is being retired with useTimelineGroupEditing.ts, so this is consistent.
CI signal on tip
| Check | Conclusion |
|---|---|
| Preflight (lint + format) | ✅ success (×3) |
| Detect changes | ✅ success (×3) |
| Preview parity | ✅ success |
| Perf: load / fps / drift / parity / scrub | ✅ success (all 5) |
| player-perf | ✅ success |
| preview-regression | ✅ success |
| regression-shards | 4/8 shards complete (all successful), 4/8 in progress |
| WIP | 🟡 in progress (not a merge blocker) |
Zero failures. Regression-shard tail + WIP are non-blocking on progress. All completed required checks green on the batch merge tip.
Nits
App.tsxhas a small inline anonymous fn fordeleteSelectedKeyframesRef.currentthat used to calldeleteSelectedKeyframes(domEditSession). The inlined version callsusePlayerStore.getState()and pullsselectedKeyframePercentagesForElement. Semantics preserved but the helper import (deleteSelectedKeyframesfromtimelineEditingHelpers) is dropped becausetimelineEditingHelpers.tsis being gutted (466 → 91 lines). Fine — the extraction moved responsibility to the store.
R1 by Via
6f9c592 to
bc7ce78
Compare
26f858b to
5872d45
Compare
bc7ce78 to
2700f84
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review — docs will 404 on merge + integration coverage regression
Two NEW findings from the deletion audit:
Docs / skills files still reference the removed export
packages/studio/src/index.ts at this PR's head no longer exports NLELayout, but three docs/skills references still target it:
docs/packages/studio.mdx:64,68,70importsNLELayoutfrom@hyperframes/studioand uses<NLELayout>in the code example. Users copy-pasting this snippet after merge get a "NLELayout is not exported" TypeScript/runtime error.docs/contributing/studio-manual-dom-editing.mdx:277references the deleted pathpackages/studio/src/components/nle/NLELayout.tsx.skills/website-to-video/references/capabilities.md:557describesNLELayoutas an active component.
Fix: update all three references in this same PR (docs example should either be removed or repointed at the new shell export; the skill capability description should be corrected).
963→290 line test coverage regression on undo/persist invariants
Deleted from hooks/useTimelineEditing.test.tsx (963 lines):
- Root-duration extension via fallback path (SDK-move + SDK-resize)
- DOM/store rollback on save-fail
- Wait-for-lifecycle before resolve
- Timing-after-z-index ordering on diagonal drag
- Same-file group move as single write
- Group move partitioned by source file with one undo entry
- Wait for z-commit before group timing write
- Single-clip-group-move ≡ single-clip-move
Added:
timelineGroupEditing.test.ts(126 lines: pure resize math)useTimelineClipDrag.resize.test.tsx(164 lines: resize preview/commit/lock-degrade)
Different surface — no replacement for the persist-ordering / undo-atomicity / DOM-rollback invariants. Miga + Via + Abhai all flagged this; noting here so the follow-up integration-test PR gets scoped correctly (batched-move / multi-delete / content-driven duration / persist-ordering are the concrete invariants to restore).
Confirmed clean
- 0 runtime dangling references at #2213 head (verified across
packages/studio/src/index.ts,App.tsx,timelineCallbacks.ts,useTimelineEditing.ts,useTimelineClipDrag.ts,timelineEditing.ts). .fallowrc.jsonccleanly drops allTEMP(studio-dnd)entries — the 37-symbol superset invariant with #2205 holds.- Multi-select resize IS present (restored from
main 36413da7f, tested atuseTimelineClipDrag.resize.test.tsx). Abhai's "doesn't exist in the new NLE" was over-called; documented cut is atomic undo (per-member persist instead of atomic multi-file commit) — same tax as multi-select move.
Additional nits
- Dead import at
useTimelineEditCallbacks.ts:63:buildDomSelectionForTimelineElementdestructured fromuseDomEditActionsContext(), appears in the deps array at:205, but never referenced in the returned callback body. Will trip lint. - Multi-select resize atomic-undo asymmetry:
useTimelineEditCallbacks.ts:87wiresonMoveElements: handleTimelineElementsMove(atomic multi-move). No matchingonResizeElements— the callback bag intimelineCallbacks.ts:26-57doesn't even declare it. Move is atomic (single undo), resize is per-member (N undos). Undocumented in changelog. Either wire anonResizeElementsbatch handler for symmetry or explicitly note the asymmetry. - TimelineEditProvider no longer refreshes on visibility toggle from Timeline:
TimelineEditContext.tsxdropsvalue.onToggleElementHiddenfrom the memo deps. Handler still exists but is routed only viaApp.tsx:503→StudioRightPanel→PropertyPanel. VerifiedTimeline.tsxandTimelineCanvas.tsxdon't reference it — intentional scope narrowing, should be called out in the PR body.
— Rames Jusso
2700f84 to
d5242de
Compare
…ngine What: the final swap (43 files): App mounts EditorShell; contexts, hotkeys, sidebar (AssetsTab→AssetCard, toolbar, audio row), useTimelineEditing and the store drop the legacy single-select API and trimmed callbacks; deletes the 7 files this orphans (StudioPreviewArea, NLELayout, useTimelineGroupEditing, timelineLayerDrag + suites, legacy hook test); .fallowrc.jsonc lands at final content, dropping every TEMP(studio-dnd) suppression — everything shipped unwired is now reachable. Why: last step of the NLE integration; after this the tree is byte-identical to the previously-green integration checkpoint. How: modified files to final content + gate-forced deletions + suppression cleanup. Tree parity with checkpoint 86dd2a32b verified. Test plan: tsc --noEmit in studio, studio-server, core; bun run --filter @hyperframes/studio build; bunx vitest run (full suite); fallow audit clean with zero remaining TEMP entries; tree-diff vs 86dd2a32b empty.
d5242de to
2e1046f
Compare
4a1b7a2 to
3e2c01f
Compare

What
the final swap (43 files): App mounts EditorShell; contexts, hotkeys, sidebar (AssetsTab→AssetCard, toolbar, audio row), useTimelineEditing and the store drop the legacy single-select API and trimmed callbacks; deletes the 7 files this orphans (StudioPreviewArea, NLELayout, useTimelineGroupEditing, timelineLayerDrag + suites, legacy hook test); .fallowrc.jsonc lands at final content, dropping every TEMP(studio-dnd) suppression — everything shipped unwired is now reachable.
Why
last step of the NLE integration; after this the tree is byte-identical to the previously-green integration checkpoint.
How
modified files to final content + gate-forced deletions + suppression cleanup. Tree parity with checkpoint 86dd2a32b verified.
Test plan
tsc --noEmit in studio, studio-server, core; bun run --filter @hyperframes/studio build; bunx vitest run (full suite); fallow audit clean with zero remaining TEMP entries; tree-diff vs 86dd2a32b empty.
Stack position 22/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary
TEMP(studio-dnd)fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.