Skip to content

Commit 2c8726e

Browse files
committed
fix(web): merge a bake or matte result over edits made while it ran
Adopting the server snapshot after an audio bake or subject isolation replaced the whole document, so a clip edit, a marker or a track deletion made during the request was lost and then marked synced. Both completions now go through the existing three-way merge against the document the action sent, scoped to the clips the route wrote, so the generated field arrives and the user's edits survive as one undo entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013in93XM9r3h7RaFz62YGwN
1 parent 67d7a98 commit 2c8726e

3 files changed

Lines changed: 280 additions & 22 deletions

File tree

web/src/stores/timeline/TimelineStore.ts

Lines changed: 153 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ import {
107107
reflowGenerated,
108108
isTranscriptClip
109109
} from "./transcriptOps";
110+
import { mergeTimelineDocuments, type TimelineMergeDoc } from "./merge";
111+
import type { DocumentOp } from "@nodetool-ai/protocol";
110112

111113
// ── Snap threshold ─────────────────────────────────────────────────────────
112114

@@ -561,7 +563,9 @@ export interface TimelineStoreState {
561563
* Drive a clip's motion from an audio clip. The measuring and the write both
562564
* happen on the server, which reads the STORED document — so this saves the
563565
* open document first, posts the bake, and takes back the document the
564-
* server wrote in one `applyAgentEdit`, i.e. one undo entry.
566+
* server wrote in one `applyAgentEdit`, i.e. one undo entry. What comes back
567+
* is merged against the document that was saved, so an edit made while the
568+
* bake ran survives instead of being overwritten by the server's copy.
565569
*/
566570
bakeAudioAnimation: (
567571
body: BakeAudioAnimationBody
@@ -589,7 +593,9 @@ export interface TimelineStoreState {
589593
* The segmentation reads the STORED document and writes the result onto the
590594
* clip, so this saves the open document first, marks the clip generating for
591595
* the editor to show, waits for the run to settle, and takes back the
592-
* document the server wrote. Resolves null when the run failed — the error
596+
* document the server wrote — merged against the document that was saved, so
597+
* an edit made while the run was in flight survives. Each round of the wait
598+
* is one undo entry. Resolves null when the run failed — the error
593599
* reaches the user as a notification rather than an unhandled rejection.
594600
*/
595601
isolateSubject: (
@@ -1267,6 +1273,105 @@ const syncedSnapshotOf = (
12671273
height: state.height
12681274
});
12691275

1276+
// ── Server-write adoption ──────────────────────────────────────────────────
1277+
1278+
/** The document as the editor last read or wrote it. */
1279+
type TimelineSyncedDoc = NonNullable<TimelineStoreState["syncedDocument"]>;
1280+
1281+
/** What `trpc.timeline.get` answers with. */
1282+
type FetchedSequence = Awaited<
1283+
ReturnType<typeof trpcClient.timeline.get.query>
1284+
>;
1285+
1286+
/**
1287+
* The write a server route made, as merge ops: it wrote the clips it was
1288+
* pointed at and nothing else. Without ops the merge engine reads the fetched
1289+
* copy as a whole-document replacement, which a dirty draft refuses whole —
1290+
* the generated field would never arrive.
1291+
*/
1292+
const clipWriteOps = (clipIds: readonly string[]): DocumentOp[] =>
1293+
clipIds.map((clipId) => ({
1294+
tool: "ui_timeline_update_clip",
1295+
input: { clip_id: clipId }
1296+
}));
1297+
1298+
/**
1299+
* Take back the document a server route wrote, keeping every edit the user
1300+
* made while the request was in flight.
1301+
*
1302+
* `base` is the document as the action saved it — the copy the server started
1303+
* from — so the three-way merge of (base, current draft, fetched copy) hands
1304+
* the generated field to the clips the route wrote and leaves every other
1305+
* local edit, addition and deletion alone. Adopting the fetched copy wholesale
1306+
* (what this replaced) dropped any edit made inside the request window,
1307+
* because `setBaseUpdatedAt` then marked the replacement as synchronized and
1308+
* autosave had nothing left to write.
1309+
*
1310+
* The whole write is one `applyAgentEdit`, i.e. one undo entry. Returns the
1311+
* base for the next adoption, so a polling caller rolls forward instead of
1312+
* merging against a copy two rounds old.
1313+
*
1314+
* The caller has already checked that the store still holds this sequence.
1315+
*/
1316+
function adoptServerSequence(
1317+
get: () => TimelineStoreState,
1318+
sequence: FetchedSequence,
1319+
base: TimelineSyncedDoc,
1320+
touchedClipIds: readonly string[]
1321+
): TimelineSyncedDoc {
1322+
const state = get();
1323+
const draft: TimelineMergeDoc = {
1324+
tracks: state.tracks,
1325+
clips: state.clips,
1326+
markers: state.markers,
1327+
transcript: state.transcript,
1328+
scriptEnabled: state.scriptEnabled,
1329+
fps: state.fps,
1330+
width: state.width,
1331+
height: state.height
1332+
};
1333+
// A field the response leaves out is one the route did not write, so the
1334+
// base stands in for it rather than reading as an external clear.
1335+
const server: TimelineMergeDoc = {
1336+
tracks: sequence.tracks ?? base.tracks,
1337+
clips: sequence.clips ?? base.clips,
1338+
markers: sequence.markers ?? base.markers,
1339+
transcript: sequence.transcript ?? base.transcript,
1340+
scriptEnabled: sequence.scriptEnabled ?? base.scriptEnabled,
1341+
fps: sequence.fps ?? base.fps,
1342+
width: sequence.width ?? base.width,
1343+
height: sequence.height ?? base.height
1344+
};
1345+
1346+
const { doc, nextBase } = mergeTimelineDocuments(
1347+
base,
1348+
draft,
1349+
server,
1350+
clipWriteOps(touchedClipIds)
1351+
);
1352+
1353+
get().applyAgentEdit({
1354+
tracks: doc.tracks as TimelineTrack[],
1355+
clips: doc.clips as TimelineClip[],
1356+
markers: doc.markers as TimelineMarker[]
1357+
});
1358+
// The base for the next external change is what the SERVER holds, minus the
1359+
// slots the draft refused, which keep the base they had — the rule
1360+
// `MergeResult.nextBase` documents.
1361+
const synced: TimelineSyncedDoc = {
1362+
tracks: nextBase.tracks as TimelineTrack[],
1363+
clips: nextBase.clips as TimelineClip[],
1364+
markers: nextBase.markers as TimelineMarker[],
1365+
transcript: nextBase.transcript as TranscriptLine[],
1366+
scriptEnabled: nextBase.scriptEnabled,
1367+
fps: nextBase.fps,
1368+
width: nextBase.width,
1369+
height: nextBase.height
1370+
};
1371+
get().setBaseUpdatedAt(sequence.updatedAt, synced);
1372+
return synced;
1373+
}
1374+
12701375
// ── Factory ────────────────────────────────────────────────────────────────
12711376

12721377
export const createTimelineStore = (
@@ -2335,6 +2440,11 @@ export const createTimelineStore = (
23352440
baseUpdatedAt: beforeSave.baseUpdatedAt ?? undefined,
23362441
document: buildTimelineDocumentPayload(beforeSave)
23372442
});
2443+
// What the server now holds, and so the base the bake's own write
2444+
// is merged against below — captured from the document that was
2445+
// SENT, not from the store after the save, which may already carry
2446+
// an edit the user made while the save was in flight.
2447+
const base = syncedSnapshotOf(beforeSave);
23382448
const savedAt = (saved as { updatedAt?: unknown } | undefined)
23392449
?.updatedAt;
23402450
if (
@@ -2353,12 +2463,9 @@ export const createTimelineStore = (
23532463
// loading this one over it is the clobber every reload path avoids.
23542464
if (get().sequenceId !== sequenceId) return result;
23552465

2356-
get().applyAgentEdit({
2357-
tracks: (sequence.tracks ?? []) as TimelineTrack[],
2358-
clips: (sequence.clips ?? []) as TimelineClip[],
2359-
markers: (sequence.markers ?? []) as TimelineMarker[]
2360-
});
2361-
get().setBaseUpdatedAt(sequence.updatedAt);
2466+
adoptServerSequence(get, sequence, base, [
2467+
result.clip_id || body.target_clip_id
2468+
]);
23622469
return result;
23632470
},
23642471

@@ -2455,23 +2562,31 @@ export const createTimelineStore = (
24552562
}));
24562563
};
24572564

2458-
/** Take back the document the server wrote, in one undo entry. */
2459-
const adopt = async (): Promise<TimelineClip | undefined> => {
2565+
/**
2566+
* Take back the document the server wrote, in one undo entry,
2567+
* merged against `base` so an edit the user made while the matte ran
2568+
* survives. Answers with the SERVER's copy of the clip — whether the
2569+
* run has settled is a question about the row, not about the draft —
2570+
* and with the base for the next round.
2571+
*/
2572+
const adopt = async (
2573+
base: TimelineSyncedDoc
2574+
): Promise<{
2575+
clip: TimelineClip | undefined;
2576+
base: TimelineSyncedDoc;
2577+
}> => {
24602578
const sequence = await trpcClient.timeline.get.query({
24612579
id: sequenceId
24622580
});
24632581
const clips = (sequence.clips ?? []) as TimelineClip[];
24642582
// The editor may have moved to another sequence while the matte
24652583
// ran; loading this one over it is the clobber every reload path
24662584
// avoids.
2467-
if (get().sequenceId !== sequenceId) return undefined;
2468-
get().applyAgentEdit({
2469-
tracks: (sequence.tracks ?? []) as TimelineTrack[],
2470-
clips,
2471-
markers: (sequence.markers ?? []) as TimelineMarker[]
2472-
});
2473-
get().setBaseUpdatedAt(sequence.updatedAt);
2474-
return clips.find((c) => c.id === clipId);
2585+
if (get().sequenceId !== sequenceId) {
2586+
return { clip: undefined, base };
2587+
}
2588+
const nextBase = adoptServerSequence(get, sequence, base, [clipId]);
2589+
return { clip: clips.find((c) => c.id === clipId), base: nextBase };
24752590
};
24762591

24772592
try {
@@ -2491,13 +2606,27 @@ export const createTimelineStore = (
24912606
}
24922607

24932608
mark("generating");
2609+
// What the server started from: the document that was SENT, plus
2610+
// the placeholder `mark` just wrote onto the clip. The placeholder
2611+
// is this action's own optimistic state, not a user edit — leaving
2612+
// it out of the base would make the clip read as edited on both
2613+
// sides, and the merge would refuse the matte the run produced.
2614+
const sent = syncedSnapshotOf(beforeSave);
2615+
const marked = get().clips.find((c) => c.id === clipId);
2616+
let base: TimelineSyncedDoc = marked
2617+
? {
2618+
...sent,
2619+
clips: sent.clips.map((c) => (c.id === clipId ? marked : c))
2620+
}
2621+
: sent;
2622+
24942623
const result = await postIsolateSubject(sequenceId, {
24952624
...body,
24962625
clip_id: clipId
24972626
});
24982627

24992628
if (result.status !== "generating") {
2500-
await adopt();
2629+
await adopt(base);
25012630
return result;
25022631
}
25032632

@@ -2508,10 +2637,12 @@ export const createTimelineStore = (
25082637
for (;;) {
25092638
await sleep(pollIntervalMs);
25102639
if (get().sequenceId !== sequenceId) return result;
2511-
const settled = await adopt();
2640+
const settled = await adopt(base);
2641+
base = settled.base;
25122642
if (
2513-
settled === undefined ||
2514-
(settled.generatedMatte?.status ?? "ready") !== "generating"
2643+
settled.clip === undefined ||
2644+
(settled.clip.generatedMatte?.status ?? "ready") !==
2645+
"generating"
25152646
) {
25162647
return result;
25172648
}

web/src/stores/timeline/__tests__/TimelineStore.audioBake.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,68 @@ describe("bakeAudioAnimation", () => {
153153
expect(store.getState().clips[0].animations).toBeUndefined();
154154
});
155155

156+
it("keeps edits made while the bake was in flight", async () => {
157+
const { store, track, target } = seedStore();
158+
// A second track with nothing on it, and a clip the bake never touches:
159+
// the two things the user is about to change while the request runs.
160+
const spare: TimelineTrack = makeTrack({ type: "video", name: "V2" });
161+
const other: TimelineClip = makeClip({
162+
id: "other-1",
163+
trackId: track.id,
164+
name: "Shot 2",
165+
startMs: 2000,
166+
durationMs: 2000,
167+
mediaType: "video"
168+
});
169+
store.setState({
170+
tracks: [track, spare],
171+
clips: [target, other]
172+
});
173+
timelineTemporalOf(store).clear();
174+
175+
// The server answers with the document as it was SAVED plus the bake —
176+
// it never saw the edits made after the save.
177+
let releaseGet: () => void = () => {};
178+
mockTimelineGet.mockImplementation(
179+
() =>
180+
new Promise((resolve) => {
181+
releaseGet = () =>
182+
resolve({
183+
id: SEQUENCE_ID,
184+
updatedAt: "2026-01-01T00:05:00.000Z",
185+
tracks: [track, spare],
186+
clips: [{ ...target, animations: [BAKED_ANIMATION] }, other],
187+
markers: []
188+
});
189+
})
190+
);
191+
192+
const pending = store.getState().bakeAudioAnimation(BODY);
193+
await new Promise((resolve) => setTimeout(resolve, 0));
194+
expect(mockTimelineGet).toHaveBeenCalledTimes(1);
195+
196+
// Inside the 750 ms autosave debounce: none of this has reached the
197+
// server, and the response in flight predates all of it.
198+
store.getState().patchClip("other-1", { name: "Renamed" });
199+
const marker = store.getState().addMarker({ timeMs: 1000, label: "Beat" });
200+
store.getState().removeTrack(spare.id);
201+
const before = timelineTemporalOf(store).pastStates.length;
202+
203+
releaseGet();
204+
await pending;
205+
206+
const state = store.getState();
207+
expect(state.clips.find((c) => c.id === "other-1")?.name).toBe("Renamed");
208+
expect(state.markers.map((m) => m.id)).toEqual([marker.id]);
209+
expect(state.tracks.map((t) => t.id)).toEqual([track.id]);
210+
// …and the bake's own write still arrived.
211+
expect(state.clips.find((c) => c.id === "target-1")?.animations).toEqual([
212+
BAKED_ANIMATION
213+
]);
214+
expect(state.baseUpdatedAt).toBe("2026-01-01T00:05:00.000Z");
215+
expect(timelineTemporalOf(store).pastStates.length).toBe(before + 1);
216+
});
217+
156218
it("leaves the document alone when the editor moved to another sequence", async () => {
157219
const { store, track, target } = seedStore();
158220
mockTimelineGet.mockImplementation(async () => {

web/src/stores/timeline/__tests__/TimelineStore.generatedMatte.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,71 @@ describe("isolateSubject", () => {
249249
expect(store.getState().clips[0].generatedMatte?.status).toBe("ready");
250250
});
251251

252+
it("keeps edits made while the matte was in flight", async () => {
253+
const { store, track, clip } = seedStore();
254+
// A second track with nothing on it, and a clip the run never touches:
255+
// the two things the user is about to change while the request runs.
256+
const spare: TimelineTrack = makeTrack({ type: "video", name: "V2" });
257+
const other: TimelineClip = makeClip({
258+
id: "other-1",
259+
trackId: track.id,
260+
name: "Shot 2",
261+
startMs: 4000,
262+
durationMs: 2000,
263+
mediaType: "video"
264+
});
265+
store.setState({ tracks: [track, spare], clips: [clip, other] });
266+
timelineTemporalOf(store).clear();
267+
268+
postIsolate.mockResolvedValue({
269+
status: "ready",
270+
assetId: "mask-2",
271+
sourceRange: { fromMs: 0, toMs: 4000 },
272+
reused: false
273+
});
274+
// The server answers with the document as it was SAVED plus the matte —
275+
// it never saw the edits made after the save.
276+
let releaseGet: () => void = () => {};
277+
mockTimelineGet.mockImplementation(
278+
() =>
279+
new Promise((resolve) => {
280+
releaseGet = () =>
281+
resolve({
282+
id: SEQUENCE_ID,
283+
updatedAt: "2026-01-01T00:05:00.000Z",
284+
tracks: [track, spare],
285+
clips: [{ ...clip, generatedMatte: MATTE }, other],
286+
markers: []
287+
});
288+
})
289+
);
290+
291+
const pending = store.getState().isolateSubject(CLIP_ID);
292+
await new Promise((resolve) => setTimeout(resolve, 0));
293+
expect(mockTimelineGet).toHaveBeenCalledTimes(1);
294+
295+
// Inside the 750 ms autosave debounce: none of this has reached the
296+
// server, and the response in flight predates all of it.
297+
store.getState().patchClip("other-1", { name: "Renamed" });
298+
const marker = store.getState().addMarker({ timeMs: 1000, label: "Beat" });
299+
store.getState().removeTrack(spare.id);
300+
const before = timelineTemporalOf(store).pastStates.length;
301+
302+
releaseGet();
303+
await pending;
304+
305+
const state = store.getState();
306+
expect(state.clips.find((c) => c.id === "other-1")?.name).toBe("Renamed");
307+
expect(state.markers.map((m) => m.id)).toEqual([marker.id]);
308+
expect(state.tracks.map((t) => t.id)).toEqual([track.id]);
309+
// …and the run's own write still arrived.
310+
expect(
311+
state.clips.find((c) => c.id === CLIP_ID)?.generatedMatte
312+
).toMatchObject({ assetId: "mask-2", status: "ready" });
313+
expect(state.baseUpdatedAt).toBe("2026-01-01T00:05:00.000Z");
314+
expect(timelineTemporalOf(store).pastStates.length).toBe(before + 1);
315+
});
316+
252317
it("reports a refusal as a notification and puts the previous result back", async () => {
253318
const { store } = seedStore(MATTE);
254319
postIsolate.mockRejectedValue(new Error("Clip carries a time remap"));

0 commit comments

Comments
 (0)