Follow-ups from reviewing #242, both deliberately left out of that PR because neither blocks it.
1. removeClip filters on clipId alone, rederiveRegionMs uses isAnchored
src/lib/ai-edition/document/timeline.ts — the cascade added in #242 drops regions with:
regions.filter((region) => region.clipId !== clipId)
but isAnchored (same file, ~line 33) requires all three of clipId / sourceStartSec / sourceEndSec, and that is the predicate rederiveRegionMs uses on the surviving-clips path. So a region carrying a bare clipId with no source range is kept when other clips survive and deleted when the last one goes.
I could not find a producer for that shape on current main — every create/paste path anchors fully, and anchorRegionsWithDerivedMs never sets clipId without a source range — so this is theoretical today. But an old on-disk document with one would lose data silently. Either mirror isAnchored in the filter, or state in the comment that a bare clipId counts as anchored for deletion purposes.
2. removeAsset has no multi-asset test
electron/ai-edition/document-service.ts — routing through removeClip is right, but it changed behaviour beyond modifier cleanup once a project has more than one asset. The old code filtered clips out in place, leaving a hole in the timeline; removeClip resequences, so clips belonging to other assets now slide left and their anchored regions get their ms rederived.
That is almost certainly correct and consistent with the UI delete path, but the test added in #242 has a single asset, so nothing pins it. Wanted: interleaved clips from two assets, remove one asset, assert the survivors' timelineStartSec and a surviving region's startMs.
Follow-ups from reviewing #242, both deliberately left out of that PR because neither blocks it.
1.
removeClipfilters onclipIdalone,rederiveRegionMsusesisAnchoredsrc/lib/ai-edition/document/timeline.ts— the cascade added in #242 drops regions with:but
isAnchored(same file, ~line 33) requires all three ofclipId/sourceStartSec/sourceEndSec, and that is the predicaterederiveRegionMsuses on the surviving-clips path. So a region carrying a bareclipIdwith no source range is kept when other clips survive and deleted when the last one goes.I could not find a producer for that shape on current
main— every create/paste path anchors fully, andanchorRegionsWithDerivedMsnever setsclipIdwithout a source range — so this is theoretical today. But an old on-disk document with one would lose data silently. Either mirrorisAnchoredin the filter, or state in the comment that a bareclipIdcounts as anchored for deletion purposes.2.
removeAssethas no multi-asset testelectron/ai-edition/document-service.ts— routing throughremoveClipis right, but it changed behaviour beyond modifier cleanup once a project has more than one asset. The old code filtered clips out in place, leaving a hole in the timeline;removeClipresequences, so clips belonging to other assets now slide left and their anchored regions get their ms rederived.That is almost certainly correct and consistent with the UI delete path, but the test added in #242 has a single asset, so nothing pins it. Wanted: interleaved clips from two assets, remove one asset, assert the survivors'
timelineStartSecand a surviving region'sstartMs.