From a9a4bae7af75c2c4f7ec7f97994cdb06cbad4679 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 4 Sep 2026 21:19:09 +0200 Subject: [PATCH 1/2] fix(insertions): the deleted word stayed in the transcript MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting an amber clip from the timeline left its generated asset and its one-word transcript in the document. The word stayed in the pane, playing nowhere, and its file stayed on disk. Only the transcript pane's delete dropped that media, and that is not the delete the user performs. The trash icon on the clip calls `removeClip`, and the assistant calls the tool of the same name; neither has ever heard of an insertion, and neither should. Same hole one lane over: the trash on a voice-over pill calls `removeAudioTrack`. So the rule goes where the deletes converge, as one sweep beside the asset drop it generalises (#350): media generated FROM a word is owned by the one clip or track that plays it, and when the last thing playing it goes, it goes. Scoped to generated ids — a recording whose last clip is deleted must stay, or "Restore full timeline" has nothing to restore. `removeGeneratedClips` and `removeGeneratedTracks` then stop doing it by hand; the first is now one line. Found while de-risking the merge: four orphaned `ext:synth_*` assets in a test project, referenced by no clip and no track. --- src/lib/ai-edition/document/audioTracks.ts | 36 ++++++++++++++++++- src/lib/ai-edition/document/insertion.test.ts | 16 +++++++++ src/lib/ai-edition/document/insertion.ts | 17 +++------ .../document/insertionTrack.test.ts | 10 +++++- src/lib/ai-edition/document/insertionTrack.ts | 13 ++++--- src/lib/ai-edition/document/timeline.ts | 12 +++++-- 6 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/lib/ai-edition/document/audioTracks.ts b/src/lib/ai-edition/document/audioTracks.ts index 19ca98964..232d72af8 100644 --- a/src/lib/ai-edition/document/audioTracks.ts +++ b/src/lib/ai-edition/document/audioTracks.ts @@ -18,6 +18,7 @@ // outer edges only. import type { AxcutAudioTrack, AxcutClip, AxcutDocument } from "../schema"; +import { isGeneratedAssetId } from "../timeline/clip-parts"; import { anchorRegionsWithDerivedMs, clampSpanAgainstNeighbours } from "../timeline/timelineMap"; /** Every fragment of one user-visible track shares this key. */ @@ -170,7 +171,40 @@ export function removeAudioTrack(doc: AxcutDocument, trackId: string): AxcutDocu audioTracks.some((t) => t.assetId === assetId) || doc.timeline.clips.some((c) => c.assetId === assetId); const assets = stillReferenced ? doc.assets : doc.assets.filter((a) => a.id !== assetId); - return { ...doc, audioTracks, assets }; + return dropUnusedGeneratedMedia({ ...doc, audioTracks, assets }); +} + +/** + * Drop the generated media nothing plays any more. + * + * The same rule as the asset drop just above, for the media an inserted word owns. That + * media is generated FROM the word: exactly one clip or one track plays it, nothing else + * can want it, and the file is remade from the text on the next save. So when the last + * thing playing it goes, it goes. + * + * It has to live here rather than in the insertion code because the delete the user + * actually performs is usually not the transcript's: the trash icon on the clip calls + * `removeClip` and the assistant calls the tool of the same name, neither of which has + * ever heard of an insertion. Leaving the asset behind left the deleted word in the + * transcript pane, playing nowhere. + * + * Scoped to generated ids on purpose. A recording whose last clip is deleted must stay in + * the document — "Restore full timeline" has to have something to restore. + */ +export function dropUnusedGeneratedMedia(doc: AxcutDocument): AxcutDocument { + const played = new Set([ + ...doc.timeline.clips.map((c) => c.assetId), + ...doc.audioTracks.map((t) => t.assetId), + ]); + const dead = (id: string) => isGeneratedAssetId(id) && !played.has(id); + if (!doc.assets.some((a) => dead(a.id)) && !doc.transcripts.some((t) => dead(t.assetId))) { + return doc; + } + return { + ...doc, + assets: doc.assets.filter((a) => !dead(a.id)), + transcripts: doc.transcripts.filter((t) => !dead(t.assetId)), + }; } /** Patch the shared payload of every fragment of one track. Payload edits (gain, diff --git a/src/lib/ai-edition/document/insertion.test.ts b/src/lib/ai-edition/document/insertion.test.ts index ec0192829..792430ba7 100644 --- a/src/lib/ai-edition/document/insertion.test.ts +++ b/src/lib/ai-edition/document/insertion.test.ts @@ -179,6 +179,22 @@ describe("removeGeneratedClips", () => { }); }); +describe("deleting the amber clip from the TIMELINE, not from the transcript", () => { + // The trash icon on the clip calls `removeClip`, and so does the agent's tool. Neither + // goes through `removeGeneratedClips`, so if the media were only dropped there, the + // deleted word would still be in the transcript pane and its file still on disk. + it("takes the generated media with it, like the transcript path does", () => { + const back = removeClip(withInsertion(), "ext:synth_1"); + expect(back.assets.map((a) => a.id)).toEqual(["a1"]); + expect(back.transcripts.map((t) => t.assetId)).toEqual(["a1"]); + }); + + it("leaves the recording's own media alone when an ordinary clip goes", () => { + const back = removeClip(withInsertion(), "ext:synth_1"); + expect(back.timeline.clips.map((c) => c.assetId)).toEqual(["a1"]); + }); +}); + describe("the join is blind to what made the clips contiguous", () => { it("also heals two halves when an ORDINARY clip between them goes", () => { // The accepted cost of the rule, on the record. Two clips of one recording whose media diff --git a/src/lib/ai-edition/document/insertion.ts b/src/lib/ai-edition/document/insertion.ts index 32a726294..14de27197 100644 --- a/src/lib/ai-edition/document/insertion.ts +++ b/src/lib/ai-edition/document/insertion.ts @@ -233,23 +233,16 @@ export function insertGeneratedClip( * Delete inserted words. * * `removeClip` does the whole of it: the gap closes, the halves rejoin when they are still - * one continuous piece of media, and the rows anchored to the half that goes away follow. - * What is left here is the media the clip was the only user of. + * one continuous piece of media, the rows anchored to the half that goes away follow, and + * the generated media nothing plays any more goes with them. Nothing is left to do here — + * the trash icon on the clip calls the same mutator, so both deletes had to end the same + * way whichever this function did. */ export function removeGeneratedClips( document: AxcutDocument, wordIds: readonly string[], ): AxcutDocument { - return wordIds.reduce((next, wordId) => { - const id = extensionAssetId(wordId); - if (!next.timeline.clips.some((c) => c.id === id)) return next; - const after = removeClip(next, id); - return { - ...after, - assets: after.assets.filter((a) => a.id !== id), - transcripts: after.transcripts.filter((t) => t.assetId !== id), - }; - }, document); + return wordIds.reduce((next, wordId) => removeClip(next, extensionAssetId(wordId)), document); } /** diff --git a/src/lib/ai-edition/document/insertionTrack.test.ts b/src/lib/ai-edition/document/insertionTrack.test.ts index cf5695464..e7fcd8fc4 100644 --- a/src/lib/ai-edition/document/insertionTrack.test.ts +++ b/src/lib/ai-edition/document/insertionTrack.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import type { AxcutDocument } from "../schema"; -import { collapseTracksToPills } from "./audioTracks"; +import { collapseTracksToPills, removeAudioTrack } from "./audioTracks"; import { insertGeneratedClip } from "./insertion"; import { insertGeneratedTrack, @@ -142,6 +142,14 @@ describe("removeGeneratedTracks", () => { expect(back.assets.some((a) => a.id === "ext:synth_1")).toBe(false); expect(back.transcripts.some((t) => t.assetId === "ext:synth_1")).toBe(false); }); + + it("does the same when the pill is deleted from the LANE instead", () => { + // The trash on the pill calls `removeAudioTrack`, which has never heard of an + // insertion. It has to end where the transcript path ends anyway. + const back = removeAudioTrack(inserted(), "ext:synth_1"); + expect(back.assets.some((a) => a.id === "ext:synth_1")).toBe(false); + expect(back.transcripts.some((t) => t.assetId === "ext:synth_1")).toBe(false); + }); }); describe("retextGeneratedTrack", () => { diff --git a/src/lib/ai-edition/document/insertionTrack.ts b/src/lib/ai-edition/document/insertionTrack.ts index eb7797124..c9a3af977 100644 --- a/src/lib/ai-edition/document/insertionTrack.ts +++ b/src/lib/ai-edition/document/insertionTrack.ts @@ -27,7 +27,12 @@ const EPS = 1e-6; import { extensionAssetId, extensionDurationSec } from "../timeline/clip-parts"; import { removedRawSpans } from "../timeline/programme-time"; import { takeProgramme } from "../timeline/take-programme"; -import { collapseTracksToPills, reanchorAudioTracks, trackGroupId } from "./audioTracks"; +import { + collapseTracksToPills, + dropUnusedGeneratedMedia, + reanchorAudioTracks, + trackGroupId, +} from "./audioTracks"; import { createId } from "./ids"; import { generatedAsset, @@ -170,12 +175,10 @@ export function removeGeneratedTracks( const pills = collapseTracksToPills(next.audioTracks) .filter((pill) => pill.assetId !== id) .map((pill) => shiftIfAfter(pill, generated.endMs, -spanMs)); - return { + return dropUnusedGeneratedMedia({ ...next, - assets: next.assets.filter((a) => a.id !== id), - transcripts: next.transcripts.filter((t) => t.assetId !== id), audioTracks: reanchorAudioTracks(pills, next.timeline.clips, () => createId("take")), - }; + }); }, document); } diff --git a/src/lib/ai-edition/document/timeline.ts b/src/lib/ai-edition/document/timeline.ts index 3bfe91fbc..8ae93098f 100644 --- a/src/lib/ai-edition/document/timeline.ts +++ b/src/lib/ai-edition/document/timeline.ts @@ -23,7 +23,12 @@ import { hasCompleteClipAnchor, } from "../timeline/timelineMap"; import { dropTrimPillsByIds, trimAppliesToClip } from "../timeline/trim-mapping"; -import { reanchorAudioTracks, removeAudioTrack, separateAudioLanes } from "./audioTracks"; +import { + dropUnusedGeneratedMedia, + reanchorAudioTracks, + removeAudioTrack, + separateAudioLanes, +} from "./audioTracks"; import { createId } from "./ids"; /** The region families a delete can target by id. Shared with the store so "which kinds @@ -1178,13 +1183,14 @@ export function removeClip(document: AxcutDocument, clipId: string): AxcutDocume // a transient wipe deleting everything), which is why the empty case is handled // here rather than left to it. if (arr.length === 0) { - return mapAllRegionCollections( + const emptied = mapAllRegionCollections( { ...next, timeline: { ...next.timeline, clips: [] } }, (regions) => regions.filter((region) => !(hasCompleteClipAnchor(region) && region.clipId === clipId)), ); + return dropUnusedGeneratedMedia(emptied); } - return withClipsChanged(next, arr); + return dropUnusedGeneratedMedia(withClipsChanged(next, arr)); } export function restoreFullTimeline(document: AxcutDocument): AxcutDocument { From b580f31d220194fbd54d866815578951623f3592 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 4 Sep 2026 22:40:54 +0200 Subject: [PATCH 2/2] test(insertions): control tests that control something The one calling itself the ordinary-clip control deleted the generated clip, so it repeated the test above it and guarded nothing. CodeRabbit caught it. Replaced with the two properties the scoping decision actually rests on: the recording survives when its LAST clip goes, which is the branch that empties the timeline and the reason `restoreFullTimeline` still has a duration to read; and generated media survives when a delete happens elsewhere while the insertion is still on the timeline. --- src/lib/ai-edition/document/insertion.test.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/lib/ai-edition/document/insertion.test.ts b/src/lib/ai-edition/document/insertion.test.ts index 792430ba7..23efb0dae 100644 --- a/src/lib/ai-edition/document/insertion.test.ts +++ b/src/lib/ai-edition/document/insertion.test.ts @@ -189,9 +189,22 @@ describe("deleting the amber clip from the TIMELINE, not from the transcript", ( expect(back.transcripts.map((t) => t.assetId)).toEqual(["a1"]); }); - it("leaves the recording's own media alone when an ordinary clip goes", () => { - const back = removeClip(withInsertion(), "ext:synth_1"); - expect(back.timeline.clips.map((c) => c.assetId)).toEqual(["a1"]); + it("leaves the recording in the document when its LAST clip goes", () => { + // Why the sweep is scoped to generated ids rather than "anything nothing plays": + // `restoreFullTimeline` reads the primary asset's duration, so emptying the + // timeline must not take the recording with it or the button has nothing to + // restore. This is the branch that empties it. + const back = removeClip(doc(), "c1"); + expect(back.timeline.clips).toEqual([]); + expect(back.assets.map((a) => a.id)).toEqual(["a1"]); + }); + + it("leaves generated media alone when an ORDINARY clip goes and the insertion stays", () => { + // The other way an over-eager sweep goes wrong: a delete somewhere else must not + // collect media that is still on the timeline. + const back = removeClip(withInsertion(), "c1"); + expect(back.timeline.clips.some((c) => c.assetId === "ext:synth_1")).toBe(true); + expect(back.assets.map((a) => a.id)).toEqual(["a1", "ext:synth_1"]); }); });