feat(studio): non-destructive crop + cross-project asset view

[miguel-heygen]

Non-destructive visual crop (persists as clip-path: inset(), one undo/step, media file untouched); every overlay surface (selection, hover, snap, marquee, handles) respects the crop. Plus the cross-project asset view (studio-server globalAssets).

Stack: media-use v2 (3/5).

---

📚 Stack — media-use v2 (split from #1923, merge bottom-up)

1. fix(producer): mix muted browser media as silent for preview-render parity #1969 · producer: muted preview↔render parity
2. feat(cli): parakeet ASR engine for transcribe + HYPERFRAMES_PYTHON override #1970 · cli: Parakeet transcribe engine
3. feat(studio): non-destructive crop + cross-project asset view #1971 · studio: non-destructive crop + asset view
4. feat(media-use): v2 media OS core + retire hyperframes-media #1972 · media-use core + retire hyperframes-media
5. feat(media-use): transcription (parakeet), transcript-cut, audio-duck #1973 · media-use edit tools (transcript-cut, audio-duck, Parakeet)

[miguel-heygen]

• feat(media-use): transcription (parakeet), transcript-cut, audio-duck #1973
• feat(media-use): v2 media OS core + retire hyperframes-media #1972
• feat(studio): non-destructive crop + cross-project asset view #1971 👈 (View in Graphite)
• feat(cli): parakeet ASR engine for transcribe + HYPERFRAMES_PYTHON override #1970
• fix(producer): mix muted browser media as silent for preview-render parity #1969
• main

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

[vanceingalls]

Verdict: LGTM — no blockers, one small nit worth a look before merge.

The scene, as I read it at cd08335d. A non-destructive crop is introduced whose persistence is nothing more elaborate than a clip-path: inset(a b c d [round r]) inline style — the very grammar CSS already renders and that the existing HTML style-patcher already round-trips (confirmed by the new fixture in packages/studio-server/src/helpers/sourceMutation.test.ts:60). No data-crop-* attribute, no bespoke renderer contract, no schema change: the compositor does the clipping natively at composite time, so pixel accuracy is inherited for free. That answers, and quietly closes, the first two runtime-interop worries I brought to the case.

Alongside it, a cross-project asset browser lands at packages/studio-server/src/routes/globalAssets.ts:22. It reads only ~/.media/manifest.jsonl, accepts no path parameters, and — the crucial detail — toPublicAsset at packages/studio-server/src/routes/globalAssets.ts:38 explicitly strips cached_path before the record ever crosses the network boundary, with an accompanying regression test at packages/studio-server/src/routes/globalAssets.test.ts:52. Path-traversal is a non-starter: there is no user input, only a fixed home-relative manifest read.

Finding-by-finding

F1 — Crop persistence contract. ✅ Verified. DomEditCropHandles.finishCropGesture at packages/studio/src/components/editor/DomEditCropHandles.tsx:144 routes through the standard onStyleCommit("clip-path", buildInsetClipPathSides(...)) — the same one every other style edit uses. buildInsetClipPathSides at packages/studio/src/components/editor/clipPathHelpers.ts:98 emits shorthand-collapsed inset() values that patchElementInHtml already understands. Zero new persistence surface; the renderer, being the browser's own compositor reading a native CSS property, requires no work at all. The claim that crop is non-destructive holds up: clip-path masks, it does not resample.

F2 — canCrop reachability across every capability leg. ✅ Verified. New field on DomEditCapabilities at packages/core/src/editing/affordances.ts:12. I walked all four return paths in resolveCapabilities (lines 109, 125, 139, 177) — every branch sets canCrop explicitly, and the tests at packages/core/src/editing/affordances.test.ts:50-69 pin the four interesting cohorts (root=false, sub-comp-host-in-master=true, script-generated=false, locked-comp=false). No dead code, no phantom branch. The useCropOverlay availability publisher (packages/studio/src/hooks/useCropMode.ts:57) ANDs on selection.capabilities.canCrop, and the store setter (packages/studio/src/player/store/playerStore.ts:259) atomically forces cropMode: false whenever availability flips off — no way to strand the UI in an armed-but-uncroppable state.

F3 — Crop math and edge clamping. ✅ Verified. resolveCropInsetFromEdgeDrag (packages/studio/src/components/editor/domEditOverlayCrop.ts:59) correctly divides deltas by scaleX/scaleY to convert overlay pixels to element pixels, and clampInset at line 54 prevents opposing sides from crossing. resolveCropInsetFromMoveDrag at line 91 keeps left+right and top+bottom totals invariant so the crop window preserves size while sliding — tested at packages/studio/src/components/editor/domEditOverlayCrop.test.ts:96, including the extreme-drag clamp case. Video-natural-size versus display-size doesn't arise here because insets are stored in authored element pixels, not video pixels — the compositor scales them alongside the element. That's the right choice.

F4 — Overlay rewrite regressions. ✅ No regressions surfaced. DomEditOverlay.tsx went from 108 lines of overlay logic to 105 lines of slightly different overlay logic; the "rewrite" reading of the +108/-105 is misleading because most is import-list churn and prop threading. Non-cropped elements skip the crop-outline child (packages/studio/src/components/editor/DomEditOverlay.tsx:761 — {cropOutlineInsetPx && (…)}), so their chrome remains the plain-border path (resolveBoxChromeClass third branch at packages/studio/src/components/editor/domEditOverlayShape.ts:36). Event-handler cleanup: DomEditCropHandles at DomEditCropHandles.tsx:92 restores the committed clip-path on unmount via effect cleanup, and useCropOverlay at useCropMode.ts:35 removes its keydown listener symmetrically. The useEffect(() => { if (!cropMode) bumpAfterExit(); }, [cropMode]) at useCropMode.ts:52 does pattern-match the "useEffect-for-state-syncing" anti-pattern from the shared rubric, but it is doing something React cannot express otherwise — forcing a re-render one frame after a sibling component's cleanup mutates the DOM style — so the escape hatch is warranted; a comment already justifies it.

F5 — Persistence race on rapid release + save. ✅ Verified safe. finishCropGesture at DomEditCropHandles.tsx:154 awaits the onStyleCommit promise chain before it re-lifts the clip, and onStyleCommit is the same synchronous-write path the rest of the panel uses — no debounce layer sits in front of a keyboard save, so a Ctrl+S the instant after pointerup queues after the commit, not around it. Cancel path (cancelCropGesture at line 162) reverts insets to gesture.startInsets without touching the source, which is what you want.

F6 — Cross-PR pull from #1972 media-use skill. ✅ None. As predicted — studio UI does not import from the media-use skill. GlobalAssetsView fetches over HTTP; the server side reads its own manifest file. Cleanly decoupled.

F7 — Small nit: hover-box legibility over a hard-clipped element. ⚠️ Non-blocking. DomEditOverlay.tsx:662 now applies hugRectForElement to the hover ring, which is correct for cropped elements — the ring hugs the visible sliver rather than the invisible bounds. Fine. But when insets fully collapse (width===0 or height===0, which the crop-math clamp explicitly permits at domEditOverlayCrop.ts:27 with Math.max(0, …)), the hover ring degenerates to a 0×0 div and disappears. Not a correctness bug — the element is invisible anyway — but a user who cropped to zero cannot un-hover-select their own element. Consider a floor of ~4px or falling back to the raw bounds when the hugged rect goes degenerate. Author call.

Verdict, restated

The persistence surface is smaller than the PR title suggests — clip-path: inset(...) is native CSS and the existing patcher already handles it, so there is nothing new for the producer or the renderer to learn. The capability plumbing is complete, the math is unit-tested including the clamp corners, the server route is read-only over a fixed path with the sensitive field stripped and a regression test to keep it stripped, and the overlay rewrite is more accurately described as an extension — the non-cropped codepath is untouched. Ship it. F7 is a suggestion; do not gate on it.

Review by Via (runtime-interop lens)

[jrusso1020]

APPROVE — part of the 5-PR media-use stack (#1969–#1973), two-lens verification. Via (runtime-interop) + a test/observability pass both clear all five, no blockers.

Verified:

• Headline "addressed" items confirmed: #1972's orphan hyperframes-media strings fixed at both call sites (sfx.mjs:125, tts.mjs:184; the only surviving ref is SKILL.md's historical changelog line, intentional); #1973's atomic writes in place on both entry points (transcribe.mjs temp+renameSync, transcript-cut.mjs encode-to-.part+rename), and the parakeet PATH→venv fallback (resolveParakeet()) is resolved.
• Test coverage solid: #1969 mutedParity.test.ts (all mute combos), #1970 parakeet.test.ts (token-merge boundaries), #1971 crop/affordance/asset-strip tests, #1973 ~0.8 test:source ratio across 4 new files.

Non-blocking follow-ups (Via-accepted nits, worth tracking, NOT gating): #1970 observability (--model ignored on the parakeet path, subprocess stderr discarded on success, no first-run model-download cue, HYPERFRAMES_PARAKEET skips the --version gate); #1973 --copy keyframe-drift warning only on stderr, not in --json.

Merge via the normal Graphite path (bottom-up, no admin-merge). Layered on Via.

— Rames Jusso

Note on CI: this PR shows Perf: drift + player-perf failing. hyperframes main has no branch protection and no perf contexts in required checks, so these are informational per the repo's convention, not a merge gate. Worth a glance that the drift is expected for the crop/asset-view change, but not blocking.

[miguel-heygen]

Landed on main (media-use v2 stack, ac0153c). The Graphite merge queue had retargeted this PR's base to a synthetic graphite-base/* branch, so GitHub couldn't auto-close it; the slice was merged to main directly. Closing.

[james-russo-rames-d-jusso]

Reviewed at 4edd01f.

Verdict: LGTM with two 🟡 concerns worth a look — no 🔴 blockers.

Layering onto Via's runtime-interop pass and James's two-lens verification, which I read first. Their F1-F6 land the load-bearing correctness questions (native clip-path: inset() persistence, no schema change, cross-project route strips cached_path, capability plumbing complete, math clamped and unit-tested, no cross-PR pull from #1972). I re-verified independently at HEAD and agree.

My angle: I focused on paths Via/James de-prioritized — the ref/effect timing inside DomEditCropHandles, the "reuse" side of "cross-project asset view" (as opposed to the "view" side), and small-surface hygiene notes. One subtle timing bug is real but transient, and one design question is worth an answer before merge.

🟡 Concerns (non-blocking, author call)

C1 — DomEditCropHandles cleanup can write the NEW selection's insets to the OLD element when a user switches selection while staying in crop mode. packages/studio/src/components/editor/DomEditCropHandles.tsx:79-86 writes committedRef.current every render based on the CURRENT state:

committedRef.current = hasCrop ? buildInsetClipPathSides(state.insets, state.radius) : null;

Then at DomEditCropHandles.tsx:91-100, the useEffect cleanup captures el = selection.element from the closure at effect-run time (the OLD element A), but reads committedRef.current at cleanup-time — which by then reflects the NEW element B's state (because the re-sync at lines 68-75 already ran during render). Sequence, if the user picks another croppable element without leaving crop mode:

1. State syncs at line 68-75 → state.insets = B's parsed insets.
2. Line 85 rewrites committedRef.current = B's insets.
3. React commits, then runs cleanup for the previous effect (dep = old selection.element). Closure has el = A. Cleanup reads committedRef.current (B's insets) and writes it to A.
4. New effect runs with el = B, lifts B's clip to none.

Net effect: element A briefly shows B's crop insets before the preview reload from the (correctly-committed) file sync catches up. File state is fine — onStyleCommit at line 155 fires with the right state.insets while still on A. So this is a cosmetic transient, not data corruption.

Whether this is worth fixing depends on how the flow actually feels. If crop-mode + selection-switch is unusual UX (users normally exit crop before reselecting), ignore. If it's a supported flow, either (a) key the component on selection.element at DomEditOverlay.tsx:551 so it re-mounts cleanly per element, or (b) capture committedRef.current into a local at the top of the effect and reference the local in cleanup, so the cleanup restores the OLD element's crop shape.

C2 — "Cross-project asset view" as promised in the PR body ("cross-project reuse") lands as view-only here. packages/studio/src/components/sidebar/GlobalAssetsView.tsx:77-88 renders each row as a plain <div> with title=... and no onClick / draggable / drag data — no way to insert or reference a global asset into the current project. The local ImageCard at AssetsTab.tsx:62-68 sets draggable + dataTransfer.setData(TIMELINE_ASSET_MIME, ...); the global rows do neither.

If this is deliberate Phase 1 shape (see what's out there, actual reuse plumbing lands in a follow-up), no blocker — just call it out in the PR body so it doesn't read as a bug. If it's an oversight (the intent was drag-into-comp from the global panel), the missing plumbing is what to add: a draggable handler + a resolution path from sha → local project asset copy.

🟢 Nits

• parseInsetClipPathSides at packages/studio/src/components/editor/clipPathHelpers.ts:76-78 rejects inset(... round Xpx Ypx) (elliptical corner radius, two-value form) by returning null. Compat-safe against Studio-authored insets (they always emit single-value radius), but if an author hand-edits HTML to elliptical radii, the panel would show 0px on all sides while the DOM has the real inset. Minor — either accept-and-drop-second-value or comment the intentional single-value contract.
• playerStore.reset() at packages/studio/src/player/store/playerStore.ts:402-425 doesn't reset cropMode or cropAvailable. In practice, the useCropOverlay availability publisher (useCropMode.ts:53-58) will fire setCropAvailable(false) on the fresh (initially-null) selection which force-clears cropMode via the store setter at playerStore.ts:262. So no actual leak — just noting that the reset list is incomplete for symmetry.
• The search input state is shared across viewMode: "local" | "global" (AssetsTab.tsx:234, used at :442 local and :511 passed to global). Typing in one scope carries over when toggling. Likely intentional (search-across-scopes), but the input is only rendered when mediaAssets.length > 0 (:418) — i.e. based on LOCAL assets — so a fresh project with zero local assets has no way to search the global scope. Minor.

✅ Independent verifies of Via's F1-F6

• F1 crop persistence contract. Confirmed DomEditCropHandles.finishCropGesture:154 routes through onStyleCommit("clip-path", buildInsetClipPathSides(...)); the same style-commit path as any other panel edit. No new persistence surface.
• F2 canCrop reachability. Confirmed at packages/core/src/editing/affordances.ts:12 and all four resolveCapabilities branches. useCropOverlay ANDs on selection.capabilities.canCrop; setCropAvailable(false) at playerStore.ts:262 atomically clears cropMode — no armed-but-uncroppable stranding.
• F3 crop math and edge clamping. resolveCropInsetFromMoveDrag at domEditOverlayCrop.ts:96-104 preserves left+right and top+bottom totals — tested at domEditOverlayCrop.test.ts:95-105, both the size-invariance case and the extreme clamp.
• F5 access control on the cross-project route. readGlobalAssets(homedir()) at packages/studio-server/src/routes/globalAssets.ts:20-33 reads only ~/.media/manifest.jsonl — no path params, no user input. toPublicAsset at :38 strips cached_path; regression-pinned at globalAssets.test.ts:44-61. One deployment-shape note: this trusts studio-server to run per-user (dev tool). Not an issue today; if this route ever ships in a shared-service deploy, the assumption is worth revisiting.

What I didn't verify

• Undo/redo across multiple crop edits interleaved with regular style edits — trusted Via's F5 that the commit path is the same synchronous-write path everywhere. Didn't drive the flow.
• Cross-format behavior on <video> vs <img> vs <audio> (audio elements have no visual so should be canCrop=false; sub-comp hosts get canCrop=true per affordances.test.ts:59, but I didn't verify the resolved live render on a real video crop).
• The bumpAfterExit useReducer trick at useCropMode.ts:63-66 — Via flagged this as a warranted escape-hatch; I didn't independently verify no alternative React idiom applies.

— Review by Rames D Jusso