fix: confirm arranged clip deletion

Author: britle20

PLANS.md

@@ -29,7 +29,7 @@ M1 should include:
 
 Use this section as the current execution order for agent work. Feature document numbering describes the planned product sequence, but actual issue order may change as dependencies, review feedback, or implementation risks become clearer.
 
-1. #48 Variable hybrid clip length -> `docs/features/19-variable-hybrid-clip-length.md`
+1. #55 Confirm before deleting clips used in arrangement
 2. #49 Adjustable arrangement length -> `docs/features/20-adjustable-arrangement-length.md`
 3. #50 Arrangement WAV export -> `docs/features/21-arrangement-wav-export.md`
 

docs/data-model.md

@@ -210,6 +210,8 @@ export interface ArrangementState {
 
 The first arrangement placement feature should create, move, select, and delete `ClipInstance` objects without mutating the source `Clip`. Deleting a placed clip from the arrangement removes only that instance. It does not delete the sidebar clip.
 
+Deleting a source clip from the sidebar removes that clip and all of its arrangement `ClipInstance` placements. The UI should require confirmation when the source clip has musical events, is an imported audio clip, or has one or more arrangement placements.
+
 The first implementation keeps arrangement tracks, clip instances, and loop range in app-level state. The data is still serializable and should map directly into future `Project.tracks`, `Project.clipInstances`, and arrangement transport fields when export/import is implemented.
 
 Default instance lengths:

src/app/App.tsx

@@ -35,6 +35,7 @@ import {
   createEmptyHybridClip,
   deleteClipInstance,
   deleteNoteEvent,
+  getClipDeleteConfirmationMessage,
   getHybridClipBarCount,
   getHybridClipLengthTicks,
   getPitchedInstrument,
@@ -1062,18 +1063,15 @@ export function App() {
       return;
     }
 
-    const hasClipData = isAudioClip(clip)
-      ? true
-      : clip.drumEvents.length > 0 || clip.noteEvents.length > 0;
+    const arrangementInstanceCount = clipInstancesRef.current.filter(
+      (instance) => instance.clipId === clipId,
+    ).length;
+    const confirmationMessage = getClipDeleteConfirmationMessage({
+      arrangementInstanceCount,
+      clip,
+    });
 
-    if (
-      hasClipData &&
-      !window.confirm(
-        isAudioClip(clip)
-          ? `Delete imported audio clip ${clip.name}?`
-          : `Delete ${clip.name} and its musical events?`,
-      )
-    ) {
+    if (confirmationMessage && !window.confirm(confirmationMessage)) {
       return;
     }
 

src/model/audio-clip.ts

@@ -35,6 +35,11 @@ export interface ImportedAudioClipDraft {
   sampleMeta: SampleMeta;
 }
 
+export interface ClipDeleteConfirmationOptions {
+  arrangementInstanceCount?: number;
+  clip: Clip;
+}
+
 const WAV_MIME_TYPES = new Set([
   "audio/wav",
   "audio/wave",
@@ -52,6 +57,39 @@ export function isHybridClip<TClip extends { kind?: string }>(
   return clip.kind === "hybrid";
 }
 
+export function getClipDeleteConfirmationMessage({
+  arrangementInstanceCount = 0,
+  clip,
+}: ClipDeleteConfirmationOptions): string | null {
+  const hasArrangementInstances = arrangementInstanceCount > 0;
+  const arrangementMessage = hasArrangementInstances
+    ? `${arrangementInstanceCount} arrangement ${arrangementInstanceCount === 1 ? "placement" : "placements"} will also be removed.`
+    : "";
+
+  if (isAudioClip(clip)) {
+    return joinConfirmationParts([
+      `Delete imported audio clip ${clip.name}?`,
+      arrangementMessage,
+    ]);
+  }
+
+  const hasMusicalEvents =
+    clip.drumEvents.length > 0 || clip.noteEvents.length > 0;
+
+  if (!hasMusicalEvents && !hasArrangementInstances) {
+    return null;
+  }
+
+  if (hasMusicalEvents) {
+    return joinConfirmationParts([
+      `Delete ${clip.name} and its musical events?`,
+      arrangementMessage,
+    ]);
+  }
+
+  return `Delete ${clip.name}? ${arrangementMessage}`;
+}
+
 export function validateImportedWavFile(file: ImportedAudioFileLike): void {
   const hasWavExtension = /\.wav$/i.test(file.name);
   const hasWavMimeType = file.type ? WAV_MIME_TYPES.has(file.type) : false;
@@ -163,3 +201,7 @@ function createUniqueId(baseId: string, existingIds: readonly string[]): string
 
   return candidate;
 }
+
+function joinConfirmationParts(parts: readonly string[]): string {
+  return parts.filter(Boolean).join(" ");
+}

src/model/index.ts

@@ -63,6 +63,7 @@ export {
   createImportedAudioClipDraft,
   createImportedAudioDisplayName,
   createImportedAudioIds,
+  getClipDeleteConfirmationMessage,
   isAudioClip,
   isHybridClip,
   validateImportedWavFile,
@@ -110,6 +111,7 @@ export type {
 export type {
   AudioClip,
   Clip,
+  ClipDeleteConfirmationOptions,
   ImportedAudioClipDraft,
   ImportedAudioFileLike,
   SampleMeta,

tests/unit/model/audio-clip.test.ts

@@ -1,9 +1,12 @@
 import { describe, expect, it } from "vitest";
 
 import {
+  createEmptyHybridClip,
   createImportedAudioClipDraft,
   createImportedAudioDisplayName,
   createImportedAudioIds,
+  getClipDeleteConfirmationMessage,
+  toggleDrumStep,
   validateImportedWavFile,
 } from "../../../src/model";
 
@@ -81,4 +84,52 @@ describe("audio clip model", () => {
       },
     });
   });
+
+  it("does not require confirmation for empty unused hybrid clips", () => {
+    expect(
+      getClipDeleteConfirmationMessage({
+        clip: createEmptyHybridClip({ id: "clip-1", name: "Clip 1" }),
+      }),
+    ).toBeNull();
+  });
+
+  it("requires confirmation for hybrid clips with musical events", () => {
+    const clip = toggleDrumStep({
+      clip: createEmptyHybridClip({ id: "clip-1", name: "Clip 1" }),
+      laneId: "kick",
+      stepIndex: 0,
+    });
+
+    expect(getClipDeleteConfirmationMessage({ clip })).toBe(
+      "Delete Clip 1 and its musical events?",
+    );
+  });
+
+  it("requires confirmation for hybrid clips used in the arrangement", () => {
+    expect(
+      getClipDeleteConfirmationMessage({
+        arrangementInstanceCount: 2,
+        clip: createEmptyHybridClip({ id: "clip-1", name: "Clip 1" }),
+      }),
+    ).toBe("Delete Clip 1? 2 arrangement placements will also be removed.");
+  });
+
+  it("includes arrangement removal in imported audio clip confirmation", () => {
+    const { clip } = createImportedAudioClipDraft({
+      clipId: "audio-clip-loop",
+      durationSeconds: 2.5,
+      fileName: "Loop.wav",
+      mimeType: "audio/wav",
+      sampleId: "imported-audio-loop",
+    });
+
+    expect(
+      getClipDeleteConfirmationMessage({
+        arrangementInstanceCount: 1,
+        clip,
+      }),
+    ).toBe(
+      "Delete imported audio clip Loop? 1 arrangement placement will also be removed.",
+    );
+  });
 });