diff --git a/docs/qa/ux7144-pr3-reliability.md b/docs/qa/ux7144-pr3-reliability.md new file mode 100644 index 000000000..496367a48 --- /dev/null +++ b/docs/qa/ux7144-pr3-reliability.md @@ -0,0 +1,28 @@ +# Edit reliability implementation evidence + +Tracks quantfive/codepress#7144. Source base: `b8995a0f3f4eca7658597b44b488ffe80929a46e` +(`codepress-main`), which already includes 0.3.13 focused-clip shortcuts and optional host history. +CodePress still pins patched 0.3.12. This source PR does not publish a package or change that pin. + +## Identified defects and coverage + +- Runtime snapshot installation reset playhead and horizontal scroll and retained removed selected IDs. It now preserves view state, clamps the playhead only at the new endpoint, and filters selection against authoritative items. +- Controller validation advanced a speculative adapter revision; an unknown transport result could be retried as a fresh delete. Validation is isolated, a pending request blocks different intent, and **Retry save** resends its immutable batch/key. A delete of B never retries an unknown delete of A. +- Late receipts replaced newer pushed revisions. Receipt adoption now checks timeline identity and revision, and observer failures cannot strand submission cleanup. +- A push during trim could overwrite the gesture. Optional `beginTrim` / `commitTrim` / `cancelTrim` capture the authoritative snapshot and original selection, consume a token once, and defer store installation until settlement. Changed authority rejects without rebasing. No-op, cancellation, duplicate commit, and pending admission are covered. +- Explicit trim intent produces existing `trim_item` and bounded `move_item` commands for normal/ripple/roll. Source frame deltas round once using speed; attachment breaks are retained. Exhausted handles fail local validation. Linked trims and synchronized cross-track ripple trims remain unsupported and reject before any mutation. + +PR2 owns pointer-hook wiring for the optional trim port. This PR exposes the port and tests it directly; the combined pointer gesture and visibility cancellation need aggregate verification. It does not infer an operation from a compound render diff on that explicit path. Other existing store actions retain their current supported-diff path. + +## Executed checks + +- Host unit suite: 106 passing tests across seven files, including 10 new regression cases and the adjusted explicit-retry case. +- Core controller/runtime/context/trim/status type-aware check: passed. +- Feature boundaries, dependency contracts, edge budgets, unused exports and class members: passed. +- Provenance verification: passed. +- Chrome browser host fixture: seven passing tests. Real mounted FreeCut surface with a deterministic host fixture, **not an authenticated CodePress backend**. Includes focused Delete/Backspace linked-cohort deletion, Meta/Control undo/redo, context menu deletion, exact-key Retry, and host textarea Space/Backspace ownership followed by timeline focus. +- Browser screenshots: `artifacts/host-delete-ripple-retry.png`, `artifacts/host-delete-ripple-menu.png`, `artifacts/host-history-meta-undone.png`, `artifacts/host-history-control-after.png`. Videos: `artifacts/pr3-browser-results/`. + +## Remaining integration evidence + +The scoped check including `editor-surface.tsx` reports unresolved `@/index.css` side-effect declarations; core changed logic passes. Full build/package/installed consumer verification and complete repository gate suite remain for aggregate delivery. No authenticated backend, real media playback/source-frame inspection, inspector/search focus matrix, or combined PR2 pointer trim is claimed here. Parent owns release version/pin/vendor patch reconciliation and aggregate package verification. The original tester usability pass remains outstanding. diff --git a/packages/freecut-editor/src/index.d.ts b/packages/freecut-editor/src/index.d.ts index 9b017f483..f1f7363b9 100644 --- a/packages/freecut-editor/src/index.d.ts +++ b/packages/freecut-editor/src/index.d.ts @@ -492,6 +492,18 @@ export interface EditorHostContextValue { } export interface HostTimelineEditPort { + beginTrim?: (itemId: string) => string | null + commitTrim?: ( + token: string, + intent: { + handle: 'start' | 'end' + deltaFrames: number + mode: 'normal' | 'ripple' | 'rolling' + itemIds: readonly string[] + neighborId?: string | null + }, + ) => Promise + cancelTrim?: (token: string) => void requestRippleDelete(itemIds: readonly string[]): Promise | void requestSetItemAttachment?(itemIds: readonly string[], rippleLinked: boolean): Promise | void } diff --git a/src/features/editor/host/context.ts b/src/features/editor/host/context.ts index 389cc080e..f11703678 100644 --- a/src/features/editor/host/context.ts +++ b/src/features/editor/host/context.ts @@ -1,3 +1,4 @@ +import type { HostTrimIntent } from './trim-intent' import { createContext, useContext } from 'react' import { isHostCapabilityEnabled, @@ -8,6 +9,9 @@ import { /** UI producer for destructive host timeline edits. */ export interface HostTimelineEditPort { + beginTrim?: (itemId: string) => string | null + commitTrim?: (token: string, intent: HostTrimIntent) => Promise + cancelTrim?: (token: string) => void /** Ask the host authority to ripple-delete the selected timeline anchors. */ requestRippleDelete(itemIds: readonly string[]): Promise | void requestSetItemAttachment?: ( diff --git a/src/features/editor/host/controller.test.ts b/src/features/editor/host/controller.test.ts index 06f9ba7d8..aa0bd9be7 100644 --- a/src/features/editor/host/controller.test.ts +++ b/src/features/editor/host/controller.test.ts @@ -32,6 +32,9 @@ import { hostSnapshotToNativeTimeline, nativeTimelineToFrameDocument } from './d import { EmbeddedEditorHostRuntime } from './runtime' import { useMediaLibraryStore } from '@/features/editor/deps/media-library' import { useTimelineStore } from '@/features/editor/deps/timeline-store' +import { useSelectionStore } from '@/shared/state/selection' +import { useTimelineSettingsStore } from '@/features/editor/deps/timeline-store' +import { trimIntentBatch } from './trim-intent' import { usePlaybackStore } from '@/shared/state/playback' const mediaReference: MediaReference = { @@ -298,7 +301,7 @@ describe('embedded FreeCut host controller', () => { await expect(controller.requestRippleDelete(['clip-1'])).rejects.toThrow( 'host transport unavailable', ) - await expect(controller.requestRippleDelete(['clip-1'])).resolves.toMatchObject({ + await expect(controller.retryPendingEdit()).resolves.toMatchObject({ status: 'applied', }) expect(submitEdit).toHaveBeenCalledTimes(2) @@ -1743,3 +1746,285 @@ describe('embedded FreeCut host controller', () => { }) }) }) + +describe('gesture authority and recovery regressions', () => { + it('keeps a newer push when an older edit receipt arrives', async () => { + const initial = snapshot() + const harness = createFakeHost(initial) + let release!: (result: HostEditResult) => void + const controller = new HostEditorController( + { + ...harness.host, + submitEdit: () => + new Promise((resolve) => { + release = resolve + }), + }, + initial, + ) + const pending = controller.submitEdit(commandForMove(initial)) + controller.replaceAuthoritativeSnapshot({ + ...initial, + timeline: { ...initial.timeline, revision: 7 }, + }) + release({ + status: 'applied', + snapshot: movedSnapshot(), + result: { status: 'applied' } as HostAppliedEditResult['result'], + }) + await pending + expect(controller.getSnapshot().timeline.revision).toBe(7) + }) + + it('retries the identical request after a lost applied receipt and newer push', async () => { + const initial = snapshot() + const harness = createFakeHost(initial) + const batches: EditCommandBatch[] = [] + const controller = new HostEditorController( + { + ...harness.host, + submitEdit: async (batch) => { + batches.push(structuredClone(batch)) + const result = await harness.host.submitEdit(batch) + if (batches.length === 1) throw new Error('lost receipt') + return result + }, + }, + initial, + ) + const batch = commandForMove(initial) + await expect(controller.submitEdit(batch)).rejects.toThrow('lost receipt') + controller.replaceAuthoritativeSnapshot(harness.getRemoteSnapshot()) + expect(controller.getTransactionState()).toBe('retry') + await expect(controller.retryPendingEdit()).resolves.toMatchObject({ status: 'replayed' }) + expect(batches[1]).toEqual(batches[0]) + expect(controller.getSnapshot().timeline.revision).toBe(1) + expect(controller.getTransactionState()).toBe('saved') + }) + + it('preserves playhead/scroll and reconciles only removed selection IDs', () => { + const initial = snapshot() + const runtime = new EmbeddedEditorHostRuntime(createFakeHost(initial).host, initial) + runtime.mountStores() + try { + usePlaybackStore.getState().setCurrentFrame(42) + useTimelineSettingsStore.getState().setScrollPosition(100) + useSelectionStore.getState().selectItems(['clip-1', 'removed']) + runtime.controller.replaceAuthoritativeSnapshot({ + ...initial, + timeline: { ...initial.timeline, revision: 1 }, + }) + expect(usePlaybackStore.getState().currentFrame).toBe(42) + expect(useTimelineSettingsStore.getState().scrollPosition).toBe(100) + expect(useSelectionStore.getState().selectedItemIds).toEqual(['clip-1']) + runtime.controller.replaceAuthoritativeSnapshot({ + ...initial, + timeline: { ...initial.timeline, revision: 2, tracks: [], durationInFrames: 20 }, + }) + expect(usePlaybackStore.getState().currentFrame).toBe(19) + expect(useSelectionStore.getState().selectedItemIds).toEqual([]) + } finally { + runtime.unmountStores() + } + }) + + it('consumes trim tokens once, cancels without mutation, and never rebases a changed gesture', async () => { + const initial = snapshot() + const harness = createFakeHost(initial) + const notices: HostNotice[] = [] + const runtime = new EmbeddedEditorHostRuntime( + { ...harness.host, notify: (notice) => notices.push(notice) }, + initial, + ) + runtime.mountStores() + const intent = { + handle: 'end' as const, + deltaFrames: -5, + mode: 'normal' as const, + itemIds: ['clip-1'], + } + try { + const canceled = runtime.beginTrim('clip-1')! + runtime.cancelTrim(canceled) + await runtime.commitTrim(canceled, intent) + const noop = runtime.beginTrim('clip-1')! + await runtime.commitTrim(noop, { ...intent, deltaFrames: 0 }) + await runtime.commitTrim(noop, intent) + expect(harness.submitEdit).not.toHaveBeenCalled() + const stale = runtime.beginTrim('clip-1')! + runtime.controller.replaceAuthoritativeSnapshot({ + ...initial, + timeline: { ...initial.timeline, revision: 1 }, + }) + // The gesture retains its original view until it settles. + await runtime.commitTrim(stale, intent) + expect(harness.submitEdit).not.toHaveBeenCalled() + expect(notices.at(-1)?.message).toContain('changed during the trim') + } finally { + runtime.unmountStores() + } + }) + + it('submits a trim once and rejects rapid overlap before losing the first intent', async () => { + const initial = snapshot() + const harness = createFakeHost(initial) + let release!: () => void + const delayed = { + ...harness.host, + submitEdit: async (batch: EditCommandBatch) => { + await new Promise((resolve) => { + release = resolve + }) + return harness.host.submitEdit(batch) + }, + } + const runtime = new EmbeddedEditorHostRuntime(delayed, initial) + runtime.mountStores() + try { + const token = runtime.beginTrim('clip-1')! + const intent = { + handle: 'end' as const, + deltaFrames: -5, + mode: 'normal' as const, + itemIds: ['clip-1'], + } + const pending = runtime.commitTrim(token, intent) + expect(runtime.beginTrim('clip-1')).toBeNull() + await runtime.commitTrim(token, intent) + release() + await pending + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + expect(runtime.controller.getSnapshot().timeline.tracks[0]!.items[0]!.durationInFrames).toBe( + 55, + ) + expect(runtime.beginTrim('clip-1')).not.toBeNull() + } finally { + runtime.unmountStores() + } + }) + + it('translates explicit start/end ripple and roll intent without diff guessing, preserving speed and attachment breaks', () => { + const initial = snapshot() + const anchor = initial.timeline.tracks[0]!.items[0]! + Object.assign(anchor, { from: 10, sourceStart: 20, sourceEnd: 140, speed: 2 }) + Object.assign(initial.timeline.tracks[0]!, { + items: [ + ...initial.timeline.tracks[0]!.items, + { ...anchor, id: 'next', from: 70, sourceStart: 0, sourceEnd: 120 }, + { ...anchor, id: 'detached', from: 130, rippleLinked: false }, + ], + }) + const batch = trimIntentBatch(initial.timeline, 'clip-1', { + handle: 'start', + deltaFrames: 5, + mode: 'ripple', + itemIds: ['clip-1'], + }) + expect(batch.commands).toMatchObject([ + { type: 'trim_item', edge: 'start', source_us: 1_000_000, timeline_us: 500_000 }, + { type: 'move_item', item_id: 'clip-1' }, + { type: 'move_item', item_id: 'next' }, + ]) + const roll = trimIntentBatch(initial.timeline, 'clip-1', { + handle: 'end', + deltaFrames: 5, + mode: 'rolling', + itemIds: ['clip-1'], + neighborId: 'next', + }) + expect(roll.commands.map((command) => command.type)).toEqual(['trim_item', 'trim_item']) + Object.assign(anchor, { linkedGroupId: 'av' }) + expect(() => + trimIntentBatch(initial.timeline, 'clip-1', { + handle: 'end', + deltaFrames: 5, + mode: 'normal', + itemIds: ['clip-1'], + }), + ).toThrow('Linked clip trimming') + }) +}) + +it('never interprets a new delete selection as a retry of an unknown earlier delete', async () => { + const initial = snapshot() + Object.assign(initial.timeline.tracks[0]!, { + items: [ + ...initial.timeline.tracks[0]!.items, + { ...initial.timeline.tracks[0]!.items[0]!, id: 'clip-2', from: 60 }, + ], + }) + const submitEdit = vi.fn(async () => { + throw new Error('unknown outcome') + }) + const controller = new HostEditorController( + { ...createFakeHost(initial).host, submitEdit }, + initial, + ) + await expect(controller.requestRippleDelete(['clip-1'])).rejects.toThrow('unknown outcome') + await expect(controller.requestRippleDelete(['clip-2'])).resolves.toMatchObject({ + status: 'unsupported', + }) + expect(submitEdit).toHaveBeenCalledTimes(1) +}) + +it('isolates transaction listeners from successful submissions and cleanup', async () => { + const initial = snapshot() + const controller = new HostEditorController(createFakeHost(initial).host, initial) + const warning = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const listener = vi.fn() + controller.subscribeTransaction(() => { + throw new Error('observer failed') + }) + controller.subscribeTransaction(listener) + try { + await expect(controller.submitEdit(commandForMove(initial))).resolves.toMatchObject({ + status: 'applied', + }) + expect(controller.getTransactionState()).toBe('saved') + expect(listener).toHaveBeenCalledTimes(2) + } finally { + warning.mockRestore() + } +}) + +it('rejects exhausted source handles before any host mutation', async () => { + const initial = snapshot() + const harness = createFakeHost(initial) + const controller = new HostEditorController(harness.host, initial) + const batch = trimIntentBatch(initial.timeline, 'clip-1', { + handle: 'end', + deltaFrames: 300, + mode: 'normal', + itemIds: ['clip-1'], + }) + await expect(controller.submitEdit(batch)).resolves.toMatchObject({ status: 'rejected' }) + expect(harness.submitEdit).not.toHaveBeenCalled() + expect(controller.getSnapshot()).toEqual(initial) +}) + +it('does not restore a superseded timeline identity from a late receipt', async () => { + const initial = snapshot() + let release!: (result: HostEditResult) => void + const controller = new HostEditorController( + { + ...createFakeHost(initial).host, + submitEdit: () => + new Promise((resolve) => { + release = resolve + }), + }, + initial, + ) + const pending = controller.submitEdit(commandForMove(initial)) + controller.replaceAuthoritativeSnapshot({ + ...initial, + timeline: { ...initial.timeline, timelineId: 'replacement', revision: 0 }, + }) + release({ + status: 'applied', + snapshot: movedSnapshot(), + result: { status: 'applied' } as HostAppliedEditResult['result'], + }) + await pending + expect(controller.getSnapshot().timeline.timelineId).toBe('replacement') +}) diff --git a/src/features/editor/host/controller.ts b/src/features/editor/host/controller.ts index 91f8c171c..b8686a953 100644 --- a/src/features/editor/host/controller.ts +++ b/src/features/editor/host/controller.ts @@ -978,6 +978,10 @@ export class HostEditorController { private readonly host: EditorHost private readonly capabilities private readonly adapter + private lastSettledBatch: EditCommandBatch | null = null + private pendingBatch: EditCommandBatch | null = null + private submission: Promise | null = null + private readonly transactionListeners = new Set<() => void>() private readonly listeners = new Set<(snapshot: EmbeddedEditorSnapshot) => void>() constructor(host: EditorHost, snapshot: EmbeddedEditorSnapshot) { @@ -1002,10 +1006,23 @@ export class HostEditorController { } replaceAuthoritativeSnapshot(snapshot: EmbeddedEditorSnapshot): void { + if ( + snapshot.project.id !== this.snapshot.project.id || + (snapshot.timeline.timelineId === this.snapshot.timeline.timelineId && + snapshot.timeline.revision < this.snapshot.timeline.revision) + ) + return this.snapshot = clone(snapshot) this.adapter.replaceDocument(hostSnapshotToControlledDocument(snapshot)) for (const listener of this.listeners) { - listener(this.getSnapshot()) + try { + listener(this.getSnapshot()) + } catch { + this.notify({ + kind: 'warning', + message: 'An editor view could not refresh. Reload the editor.', + }) + } } } @@ -1049,7 +1066,64 @@ export class HostEditorController { return this.submitEdit(derived.batch) } - async submitEdit(batch: EditCommandBatch): Promise { + getTransactionState = (): 'saving' | 'retry' | 'saved' => + this.submission ? 'saving' : this.pendingBatch ? 'retry' : 'saved' + + subscribeTransaction = (listener: () => void): (() => void) => { + this.transactionListeners.add(listener) + return () => this.transactionListeners.delete(listener) + } + + private transactionChanged(): void { + for (const listener of this.transactionListeners) { + try { + listener() + } catch { + console.warn('Host transaction listener failed') + } + } + } + + retryPendingEdit = (): Promise => { + if (this.submission) return this.submission + if (!this.pendingBatch) + return Promise.resolve({ + status: 'unsupported', + snapshot: this.getSnapshot(), + reason: 'There is no edit to retry', + }) + return this.submitEdit(this.pendingBatch) + } + + submitEdit(batch: EditCommandBatch): Promise { + if (this.pendingBatch && stableSerialize(batch) !== stableSerialize(this.pendingBatch)) { + const reason = this.submission + ? 'Saving the current edit. Try this edit again once it is saved.' + : 'The previous edit may have saved. Retry it to confirm before making another edit.' + this.notify({ kind: 'warning', message: reason }) + return Promise.resolve({ status: 'unsupported', snapshot: this.getSnapshot(), reason }) + } + if (this.submission) return this.submission + const retry = + this.pendingBatch !== null || + (this.lastSettledBatch !== null && + stableSerialize(batch) === stableSerialize(this.lastSettledBatch)) + const promise = this.performSubmit(clone(batch), retry) + this.submission = promise + this.transactionChanged() + void promise + .finally(() => { + this.submission = null + this.transactionChanged() + }) + .catch(() => undefined) + return promise + } + + private async performSubmit( + batch: EditCommandBatch, + retry: boolean, + ): Promise { const unsupported = batch.commands.find((command) => { const capability = capabilityForCommand(command.type) return !capability || !isHostCapabilityEnabled(this.capabilities, capability) @@ -1060,8 +1134,14 @@ export class HostEditorController { return { status: 'unsupported', snapshot: this.getSnapshot(), reason } } - const localResult = this.adapter.apply(batch) - if (localResult.status === 'rejected') { + // Validation never advances the authoritative adapter. A fresh validator + // prevents a second request from seeing a speculative revision. + const localResult = retry + ? null + : createCodePressCommandAdapter({ + document: this.adapter.getDocument(), + }).apply(batch) + if (localResult?.status === 'rejected') { this.notify({ kind: 'error', message: localResult.error.message, @@ -1070,17 +1150,18 @@ export class HostEditorController { return { status: 'rejected', snapshot: this.getSnapshot(), result: localResult } } - let remoteResult: HostEditResult - try { - remoteResult = await this.host.submitEdit(batch) - } catch (error) { - // The private adapter is only a validation aid. A transport failure has - // no authoritative receipt, so discard its speculative revision before - // allowing a retry derived from the unchanged host snapshot. - this.adapter.replaceDocument(freeCutDocumentToControlledDocument(this.snapshot.timeline)) - throw error + // An unknown transport outcome must retain the identical operation and + // idempotency key even if a newer snapshot arrives before Retry. + this.pendingBatch = clone(batch) + const remoteResult = await this.host.submitEdit(clone(batch)) + this.pendingBatch = null + this.lastSettledBatch = batch + if ( + this.snapshot.timeline.timelineId === batch.timeline_id && + remoteResult.snapshot.timeline.timelineId === batch.timeline_id + ) { + this.replaceAuthoritativeSnapshot(remoteResult.snapshot) } - this.replaceAuthoritativeSnapshot(remoteResult.snapshot) if (remoteResult.status === 'conflict') { this.notify({ kind: 'conflict', diff --git a/src/features/editor/host/edit-status.tsx b/src/features/editor/host/edit-status.tsx new file mode 100644 index 000000000..851c1268f --- /dev/null +++ b/src/features/editor/host/edit-status.tsx @@ -0,0 +1,36 @@ +import { useSyncExternalStore } from 'react' +import type { HostEditorController } from './controller' + +/** Recovery retries the exact request, including after an unknown save outcome. */ +export function HostEditStatus({ controller }: { controller: HostEditorController }) { + const state = useSyncExternalStore( + controller.subscribeTransaction, + controller.getTransactionState, + controller.getTransactionState, + ) + return ( +
+ {state === 'saving' + ? 'Saving…' + : state === 'retry' + ? 'Couldn’t confirm save. Retry before making another edit.' + : 'Saved'} + {state === 'retry' && ( + + )} +
+ ) +} diff --git a/src/features/editor/host/editor-surface.tsx b/src/features/editor/host/editor-surface.tsx index be5aae1fb..13e096fde 100644 --- a/src/features/editor/host/editor-surface.tsx +++ b/src/features/editor/host/editor-surface.tsx @@ -1,3 +1,4 @@ +import { HostEditStatus } from './edit-status' import { useEffect, useState } from 'react' import { I18nextProvider } from 'react-i18next' import { TooltipProvider } from '@/components/ui/tooltip' @@ -127,6 +128,9 @@ export function FreeCutEditorSurface({ host }: { host: EditorHost }) { capabilities, host, timeline: { + beginTrim: state.runtime.beginTrim, + commitTrim: state.runtime.commitTrim, + cancelTrim: state.runtime.cancelTrim, requestRippleDelete: state.runtime.requestRippleDelete, requestSetItemAttachment: (itemIds, rippleLinked) => state.runtime.requestSetItemAttachment(itemIds, rippleLinked), @@ -136,18 +140,21 @@ export function FreeCutEditorSurface({ host }: { host: EditorHost }) { -
- +
+ +
+ +
diff --git a/src/features/editor/host/runtime.ts b/src/features/editor/host/runtime.ts index b59871659..b153d96f1 100644 --- a/src/features/editor/host/runtime.ts +++ b/src/features/editor/host/runtime.ts @@ -1,3 +1,4 @@ +import { trimIntentBatch, type HostTrimIntent } from './trim-intent' import { installRuntimeMediaResolver, useMediaLibraryStore, @@ -55,6 +56,13 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr readonly host: EditorHost readonly projectId: string + private trimGesture: { + token: string + itemId: string + snapshot: EmbeddedEditorSnapshot + itemIds: string[] + } | null = null + private installedSnapshot = false private mounted = false private applyingAuthoritative = false private reconcileScheduled = false @@ -74,6 +82,73 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr this.controller = new HostEditorController(host, snapshot) } + readonly beginTrim = (itemId: string): string | null => { + if (!this.mounted || this.trimGesture || this.controller.getTransactionState() !== 'saved') { + this.host.notify?.({ + kind: 'info', + message: 'Finish saving or retry the pending edit before trimming.', + }) + return null + } + const snapshot = this.controller.getSnapshot() + if (!snapshot.timeline.tracks.some((track) => track.items.some((item) => item.id === itemId))) + return null + const selected = useSelectionStore.getState().selectedItemIds + const itemIds = selected.includes(itemId) ? [...selected] : [itemId] + const token = crypto.randomUUID() + this.trimGesture = { token, itemId, snapshot, itemIds } + return token + } + + readonly cancelTrim = (token: string): void => { + if (this.trimGesture?.token !== token) return + this.trimGesture = null + if (this.mounted) this.applySnapshotToStores(this.authoritativeSnapshot) + } + + readonly commitTrim = async (token: string, intent: HostTrimIntent): Promise => { + const gesture = this.trimGesture + if (!gesture || gesture.token !== token || !this.mounted) return + this.trimGesture = null // one-shot, including no-op and rejection + try { + if (intent.deltaFrames === 0) return + await this.submitTrimGesture(gesture, intent) + } catch (error) { + this.host.notify?.({ + kind: 'error', + message: + error instanceof Error + ? error.message + : 'The trim could not be saved. Retry the pending edit.', + }) + } finally { + if (this.mounted) this.applySnapshotToStores(this.authoritativeSnapshot) + } + } + + private async submitTrimGesture( + gesture: NonNullable, + intent: HostTrimIntent, + ): Promise { + const current = this.controller.getSnapshot().timeline + if ( + current.timelineId !== gesture.snapshot.timeline.timelineId || + current.revision !== gesture.snapshot.timeline.revision || + JSON.stringify(current) !== JSON.stringify(gesture.snapshot.timeline) || + JSON.stringify(this.controller.getSnapshot().assets) !== + JSON.stringify(gesture.snapshot.assets) + ) { + throw new Error('This clip changed during the trim. Review the updated cut and try again.') + } + const batch = trimIntentBatch(gesture.snapshot.timeline, gesture.itemId, { + ...intent, + itemIds: gesture.itemIds, + }) + const result = await this.controller.submitEdit(batch) + if (result.status === 'unsupported') + this.host.notify?.({ kind: 'warning', message: result.reason }) + } + /** * Runtime producer for the host UI's Delete action. It submits against the * controller's authoritative snapshot and leaves the visible stores alone @@ -175,7 +250,7 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr this.unsubscribeController = this.controller.subscribe((snapshot) => { this.authoritativeSnapshot = snapshot - this.applySnapshotToStores(snapshot) + if (!this.trimGesture) this.applySnapshotToStores(snapshot) }) this.unsubscribeTimeline = useTimelineStore.subscribe(() => this.scheduleReconcile()) this.unsubscribePlayback = usePlaybackStore.subscribe((state, previous) => { @@ -186,6 +261,8 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr unmountStores(): void { if (!this.mounted) return this.mounted = false + this.trimGesture = null + this.installedSnapshot = false if (this.gestureListenersAttached) { document.removeEventListener('pointerdown', this.resumePreviewAudioOnGesture) document.removeEventListener('keydown', this.resumePreviewAudioOnGesture) @@ -217,6 +294,10 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr private applySnapshotToStores(snapshot: EmbeddedEditorSnapshot): void { const native = hostSnapshotToNativeTimeline(snapshot) + const firstInstall = !this.installedSnapshot + this.installedSnapshot = true + const selected = useSelectionStore.getState().selectedItemIds + const currentFrame = usePlaybackStore.getState().currentFrame this.applyingAuthoritative = true try { // Skim overlays are module-global UI state. Never let a media-card hover @@ -271,22 +352,31 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr useCompositionsStore.getState().setCompositions([]) useCompositionNavigationStore.getState().resetToRoot() useTimelineSettingsStore.getState().setFps(native.fps) - useTimelineSettingsStore.getState().setScrollPosition(0) + if (firstInstall) useTimelineSettingsStore.getState().setScrollPosition(0) useTimelineSettingsStore.getState().setTimelineLoading(false) useTimelineSettingsStore.getState().markClean() useTimelineStore.temporal.getState().clear() - usePlaybackStore.getState().setCurrentFrame(0) + const existingIds = new Set(native.items.map((item) => item.id)) + useSelectionStore.getState().selectItems(selected.filter((id) => existingIds.has(id))) + usePlaybackStore + .getState() + .setCurrentFrame( + firstInstall + ? 0 + : Math.min(currentFrame, Math.max(0, snapshot.timeline.durationInFrames - 1)), + ) } finally { this.applyingAuthoritative = false } } private scheduleReconcile(): void { - if (!this.mounted || this.applyingAuthoritative || this.reconcileScheduled) return + if (!this.mounted || this.applyingAuthoritative || this.trimGesture || this.reconcileScheduled) + return this.reconcileScheduled = true scheduleMicrotask(() => { this.reconcileScheduled = false - if (!this.mounted || this.applyingAuthoritative) return + if (!this.mounted || this.applyingAuthoritative || this.trimGesture) return void this.reconcileTimeline() }) } diff --git a/src/features/editor/host/trim-intent.ts b/src/features/editor/host/trim-intent.ts new file mode 100644 index 000000000..0d9eb2f0a --- /dev/null +++ b/src/features/editor/host/trim-intent.ts @@ -0,0 +1,166 @@ +import type { + EditCommand, + EditCommandBatch, + FreeCutFrameDocument, + FreeCutFrameItem, +} from '@/features/editor/codepress' +import { framesToMicroseconds } from '@/features/editor/codepress/timing' + +/** Frame deltas are quantized once, at the command boundary. */ +export interface HostTrimIntent { + handle: 'start' | 'end' + deltaFrames: number + mode: 'normal' | 'ripple' | 'rolling' + itemIds: readonly string[] + neighborId?: string | null +} + +function assertTrimEditable(document: FreeCutFrameDocument, item: FreeCutFrameItem): void { + if (item.linkedGroupId) + throw new Error('Linked clip trimming is not supported by the host yet. No clips were changed.') + let track = document.tracks.find((candidate) => candidate.id === item.trackId) + const seen = new Set() + while (track && !seen.has(track.id)) { + seen.add(track.id) + if (track.locked) + throw new Error('Unlock the affected track before trimming. No clips were changed.') + track = document.tracks.find((candidate) => candidate.id === track?.parentTrackId) + } +} + +function trimmedSourceFrame( + item: FreeCutFrameItem, + edge: HostTrimIntent['handle'], + deltaFrames: number, +): number { + if (item.type !== 'video' && item.type !== 'audio' && item.type !== 'image') + throw new Error('This item does not support source trimming.') + const sourceStart = item.sourceStart ?? 0 + const sourceEnd = item.sourceEnd ?? sourceStart + item.durationInFrames + const speed = item.speed ?? 1 + const source = (edge === 'start' ? sourceStart : sourceEnd) + Math.round(deltaFrames * speed) + if (source < 0) throw new Error('This trim exceeds the available source handles.') + return source +} + +function rollingNeighbor( + all: readonly FreeCutFrameItem[], + anchor: FreeCutFrameItem, + intent: HostTrimIntent, +): FreeCutFrameItem { + const neighbor = all.find((item) => item.id === intent.neighborId) + if ( + !neighbor || + neighbor.trackId !== anchor.trackId || + (intent.handle === 'end' + ? anchor.from + anchor.durationInFrames !== neighbor.from + : neighbor.from + neighbor.durationInFrames !== anchor.from) + ) { + throw new Error('The adjacent cut changed. Review the cut and try again.') + } + return neighbor +} + +function appendRippleMoves( + document: FreeCutFrameDocument, + anchor: FreeCutFrameItem, + intent: HostTrimIntent, + move: (item: FreeCutFrameItem, from: number) => void, +): void { + if (document.tracks.some((track) => track.id !== anchor.trackId && track.syncLock)) { + throw new Error( + 'This trim affects synchronized tracks. Use a supported sequence edit; no clips were changed.', + ) + } + const shift = intent.handle === 'start' ? -intent.deltaFrames : intent.deltaFrames + if (intent.handle === 'start') move(anchor, anchor.from) + // Respect durable attachment breaks, including a detached anchor. Gaps + // before the break retain their width, matching the existing ripple tool. + if (anchor.rippleLinked !== false) { + const tail = document.tracks + .flatMap((track) => track.items) + .filter( + (item) => + item.trackId === anchor.trackId && item.from >= anchor.from + anchor.durationInFrames, + ) + .sort((a, b) => a.from - b.from) + for (const item of tail) { + if (item.rippleLinked === false) break + move(item, item.from + shift) + } + } +} + +export function trimIntentBatch( + document: FreeCutFrameDocument, + anchorId: string, + intent: HostTrimIntent, +): EditCommandBatch { + const all = document.tracks.flatMap((track) => track.items) + const anchor = all.find((item) => item.id === anchorId) + if (!anchor) throw new Error('This clip was removed. Select a current clip and try again.') + if (!Number.isSafeInteger(intent.deltaFrames)) + throw new Error('The trim must end on a timeline frame.') + const ids = new Set(intent.itemIds) + if (ids.size !== 1 || !ids.has(anchorId)) + throw new Error('Trim one clip at a time. The selected cohort was not changed.') + const commands: EditCommand[] = [] + const touched = new Map() + const assertEditable = (item: FreeCutFrameItem) => { + assertTrimEditable(document, item) + touched.set(item.id, item) + } + const trim = (item: FreeCutFrameItem, edge: 'start' | 'end') => { + assertEditable(item) + const delta = intent.deltaFrames + const source = trimmedSourceFrame(item, edge, delta) + commands.push({ + command_id: `trim-${item.id}`, + type: 'trim_item', + item_id: item.id, + edge, + timeline_us: framesToMicroseconds( + (edge === 'start' ? item.from : item.from + item.durationInFrames) + delta, + document.fps, + ), + source_us: framesToMicroseconds(source, document.fps), + }) + } + const move = (item: FreeCutFrameItem, from: number) => { + assertEditable(item) + commands.push({ + command_id: `move-${item.id}`, + type: 'move_item', + item_id: item.id, + to_track_id: item.trackId, + timeline_start_us: framesToMicroseconds(from, document.fps), + index: document.tracks + .find((track) => track.id === item.trackId)! + .items.findIndex((candidate) => candidate.id === item.id), + }) + } + trim(anchor, intent.handle) + if (intent.mode === 'rolling') { + const neighbor = rollingNeighbor(all, anchor, intent) + trim(neighbor, intent.handle === 'start' ? 'end' : 'start') + } else if (intent.mode === 'ripple') { + appendRippleMoves(document, anchor, intent, move) + } + if (commands.length > 64) + throw new Error('This trim exceeds the host operation limit. No clips were changed.') + return { + contract_version: 1, + timeline_id: document.timelineId, + base_revision: document.revision, + operation_id: `op-${crypto.randomUUID()}`, + idempotency_key: `idem-${crypto.randomUUID()}`, + commands, + preconditions: [...touched.values()].map((item) => ({ + type: 'item_at' as const, + item_id: item.id, + track_id: item.trackId, + timeline_start_us: framesToMicroseconds(item.from, document.fps), + timeline_end_us: framesToMicroseconds(item.from + item.durationInFrames, document.fps), + })), + } +} diff --git a/tests/browser/host-delete-ripple.spec.ts b/tests/browser/host-delete-ripple.spec.ts index 1c96d44d4..c29e612cf 100644 --- a/tests/browser/host-delete-ripple.spec.ts +++ b/tests/browser/host-delete-ripple.spec.ts @@ -150,19 +150,39 @@ test.describe('host authoritative delete/ripple', () => { const rejectedBatch = await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch(), ) - // Reacquire selection/focus after the rejected authoritative receipt. - const retryClip = page.locator('[data-timeline-item][data-item-id="video-1"]') - await retryClip.click() - await expect(retryClip).toBeFocused() - await expect(retryClip).toHaveAttribute('aria-pressed', 'true') - await page.keyboard.press('Delete') - await page.waitForFunction( - (previousId) => - window.__freecutDeleteRippleFixture.getLastBatch()?.idempotency_key !== previousId, - rejectedBatch?.idempotency_key, + await page.getByRole('button', { name: 'Retry save' }).click() + await expect(page.getByTestId('host-edit-status')).toContainText('Saving') + expect(await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch())).toEqual( + rejectedBatch, ) await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseReceipt()) await expect(page.locator('[data-timeline-item="true"][data-item-id="video-1"]')).toHaveCount(0) await page.screenshot({ path: 'artifacts/host-delete-ripple-retry.png', fullPage: true }) }) }) + +test('host chat text owns Space and Backspace, then timeline click restores Delete', async ({ + page, +}) => { + await page.goto('/tests/browser/host-delete-ripple.html') + const clip = page.locator('[data-timeline-item][data-item-id="video-1"]') + await clip.click() + await page.evaluate(() => { + const input = document.createElement('textarea') + input.setAttribute('aria-label', 'Fixture host chat') + input.style.cssText = 'position:fixed;top:0;left:0;z-index:99999' + document.body.append(input) + input.focus() + }) + const chat = page.getByRole('textbox', { name: 'Fixture host chat' }) + await chat.fill('hello') + await page.keyboard.press('Space') + await page.keyboard.press('Backspace') + await expect(chat).toHaveValue('hello') + expect(await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch())).toBeNull() + await clip.click() + await page.keyboard.press('Backspace') + await page.waitForFunction(() => Boolean(window.__freecutDeleteRippleFixture.getLastBatch())) + await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseReceipt()) + await expect(clip).toHaveCount(0) +})