fix: gate studio timeline actions by capability - #415
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Approving. The capability model is the right abstraction for this problem — previously canOffsetTrimClipStart was the only gate and every other timeline action assumed patchability implicitly; centralizing that into getTimelineEditCapabilities and then wiring cursor / handle opacity / handle pointer-events / drag + resize handlers through the same struct makes the rules discoverable and the UI honest. Tests cover the three meaningful variants. Clean PR.
A few staff-lens observations, in rough order of impact — none are blockers:
1. Double capability computation across Timeline.tsx and TimelineClip.tsx.
Both files call getTimelineEditCapabilities(el) per render — Timeline inside els.map() (L1085ish), TimelineClip inside its render (L63). Prefer computing once in Timeline and passing capabilities as a prop into TimelineClip. Two wins: avoids redundant work per-clip per-frame, and removes the risk of the two call sites drifting if the function ever grows a memoization/options parameter. Minor cleanup, easy to fold in.
2. Input shape duplicates a subset of TimelineElement.
The structural input to getTimelineEditCapabilities, hasPatchableTimelineTarget, and canOffsetTrimClipStart is a hand-written mirror of { tag, duration, domId?, selector?, playbackStart?, playbackStartAttr?, sourceDuration? }. Since every production caller passes a TimelineElement, typing the input as Pick<TimelineElement, "tag" | "duration" | "domId" | "selector" | "playbackStart" | "playbackStartAttr" | "sourceDuration"> (or just TimelineElement outright) auto-picks up new fields when the store grows — and, in particular, means a future field like a new trim-eligibility signal can't silently get forgotten here. Tests can still pass plain objects because Pick/TimelineElement fields are all optional except tag/duration.
3. API surface — canOffsetTrimClipStart vs getTimelineEditCapabilities side-by-side.
Now that the composite function is the documented entry point, canOffsetTrimClipStart is really an internal detail of "can we trim the start?". Leaving it as a peer export invites a future maintainer to gate on it directly and miss the canPatch && hasFiniteDuration preconditions. Consider @internal jsdoc, or un-exporting it and inlining into getTimelineEditCapabilities (the test for it would move to testing that capability slice of the composite function — arguably more meaningful anyway).
4. Silent semantic change on end-trim: now requires finite, positive duration.
Before this PR, end-trim pointer-events were gated only on onResizeStart, with handle opacity gated on showHandles. After this PR, both go through capabilities.canTrimEnd which adds Number.isFinite(duration) && duration > 0. For any clip with duration === Infinity or 0 that previously exposed an end-trim affordance, this silently removes it. Likely intentional (you can't write a new data-duration from nothing) — I'd just call it out explicitly in the PR body so the next person hunting a regression has the receipt. The current body says "end trim is valid when Studio can patch data-duration" which hides the finite-duration precondition under the noun phrase.
5. Test coverage gaps (low priority, fold in only if easy):
duration === 0andduration === Infinityon an otherwise patchable clip → assertscanTrimEnd: false,canTrimStart: false. The> 0andisFiniteguards are stricter than the prior behavior — one test each locks that semantic in.- Clip with
domIdbut notag/ invalidtag→ confirmshasPatchableTimelineTargetalone isn't enough for start-trim. - Combo with
canOffsetTrimClipStart's existing media-source-duration branch, to show it still participates in the composite.
Non-issues I checked:
TimelineElementonmainhasdomId/selector/playbackStartAttr/sourceDuration/playbackRateas optional fields — good, the runtimeelactually carries these. (I was briefly worried until I re-fetched main.)- The cursor change in
TimelineClip(capabilities.canMove ? "grab" : "default") is consistent with the pointer-down early-return inTimeline.tsx— no UX dead zone where the cursor says "grab" but the clip won't drag. - Existing
resolveTimelineResizemath is untouched; this PR is purely gating.
Review by Rames Jusso
Merge activity
|
## Summary - gate timeline actions to clips Studio can control deterministically - disable direct move/trim for generic GSAP-timed DOM clips - add an in-clip `Copy to Agent` fallback for unsupported edits ## Why Studio should only advertise timeline actions it can round-trip to source HTML with deterministic meaning. This PR now follows that stricter rule: - direct move/end-trim are only exposed for clips with a deterministic timeline window - start trim is only exposed for clips with a real content-offset model - unsupported motion clips now offer `Copy to Agent` so users still have a fast path to request source-level timing changes In practice this means generic GSAP-authored DOM clips no longer pretend Studio can rewrite their visible timing just by patching `data-start` / `data-duration`. ## What changed - added `hasPatchableTimelineTarget()` and `getTimelineEditCapabilities()` in `timelineEditing.ts` - tightened deterministic-window detection so only media, images, and composition hosts keep direct move/end-trim controls - kept wrapped media clips editable by recognizing real media metadata even when the host tag is a `div` - updated `TimelineClip` / `Timeline` to guard interactions with the shared capability model - added `buildTimelineElementAgentPrompt()` and a `Copy to Agent` fallback button for unsupported clips - added focused tests for capability derivation and the agent-prompt helper ## Verification ### Automated - `bun test packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/Timeline.test.ts packages/studio/src/player/store/playerStore.test.ts packages/studio/src/utils/sourcePatcher.test.ts` - `bun run --filter @hyperframes/studio typecheck` - `bunx oxlint packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsx` - `bunx oxfmt --check packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsx` ### Browser Verified in Studio with live browser automation against `http://127.0.0.1:4175/#project/timeline-edit-playground`: - generic GSAP-timed clips (`feature-card`, `title-card`, `prompt-card`) show `Copy to Agent` and no direct move/trim affordances - wrapped media (`media-card`) still exposes direct controls and remains draggable - the local playground timings were realigned to match the authored GSAP positions, so preview visibility now matches the timeline windows during manual testing Recording artifacts used during verification: - `/tmp/timeline-capabilities-proof/capabilities-flow.webm` - `/tmp/timeline-capabilities-proof/capabilities-agent-flow.webm`
## Summary - gate timeline actions to clips Studio can control deterministically - disable direct move/trim for generic GSAP-timed DOM clips - add an in-clip `Copy to Agent` fallback for unsupported edits ## Why Studio should only advertise timeline actions it can round-trip to source HTML with deterministic meaning. This PR now follows that stricter rule: - direct move/end-trim are only exposed for clips with a deterministic timeline window - start trim is only exposed for clips with a real content-offset model - unsupported motion clips now offer `Copy to Agent` so users still have a fast path to request source-level timing changes In practice this means generic GSAP-authored DOM clips no longer pretend Studio can rewrite their visible timing just by patching `data-start` / `data-duration`. ## What changed - added `hasPatchableTimelineTarget()` and `getTimelineEditCapabilities()` in `timelineEditing.ts` - tightened deterministic-window detection so only media, images, and composition hosts keep direct move/end-trim controls - kept wrapped media clips editable by recognizing real media metadata even when the host tag is a `div` - updated `TimelineClip` / `Timeline` to guard interactions with the shared capability model - added `buildTimelineElementAgentPrompt()` and a `Copy to Agent` fallback button for unsupported clips - added focused tests for capability derivation and the agent-prompt helper ## Verification ### Automated - `bun test packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/Timeline.test.ts packages/studio/src/player/store/playerStore.test.ts packages/studio/src/utils/sourcePatcher.test.ts` - `bun run --filter @hyperframes/studio typecheck` - `bunx oxlint packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsx` - `bunx oxfmt --check packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsx` ### Browser Verified in Studio with live browser automation against `http://127.0.0.1:4175/#project/timeline-edit-playground`: - generic GSAP-timed clips (`feature-card`, `title-card`, `prompt-card`) show `Copy to Agent` and no direct move/trim affordances - wrapped media (`media-card`) still exposes direct controls and remains draggable - the local playground timings were realigned to match the authored GSAP positions, so preview visibility now matches the timeline windows during manual testing Recording artifacts used during verification: - `/tmp/timeline-capabilities-proof/capabilities-flow.webm` - `/tmp/timeline-capabilities-proof/capabilities-agent-flow.webm`
Summary
Copy to Agentfallback for unsupported editsWhy
Studio should only advertise timeline actions it can round-trip to source HTML with deterministic meaning.
This PR now follows that stricter rule:
Copy to Agentso users still have a fast path to request source-level timing changesIn practice this means generic GSAP-authored DOM clips no longer pretend Studio can rewrite their visible timing just by patching
data-start/data-duration.What changed
hasPatchableTimelineTarget()andgetTimelineEditCapabilities()intimelineEditing.tsdivTimelineClip/Timelineto guard interactions with the shared capability modelbuildTimelineElementAgentPrompt()and aCopy to Agentfallback button for unsupported clipsVerification
Automated
bun test packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/Timeline.test.ts packages/studio/src/player/store/playerStore.test.ts packages/studio/src/utils/sourcePatcher.test.tsbun run --filter @hyperframes/studio typecheckbunx oxlint packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsxbunx oxfmt --check packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/TimelineClip.tsxBrowser
Verified in Studio with live browser automation against
http://127.0.0.1:4175/#project/timeline-edit-playground:feature-card,title-card,prompt-card) showCopy to Agentand no direct move/trim affordancesmedia-card) still exposes direct controls and remains draggableRecording artifacts used during verification:
/tmp/timeline-capabilities-proof/capabilities-flow.webm/tmp/timeline-capabilities-proof/capabilities-agent-flow.webm