Implement DELETE /api/speakers/{index}; the Delete button did nothing

The speakers page has carried a Delete button that called
DELETE /api/speakers/{index}. That endpoint did not exist - the server
answered 405 Method Not Allowed and the button silently did nothing.

Deleting is more than dropping an entry, so the endpoint refuses to orphan
dialogue: a speaker with utterances returns 409 with the count and the list
of possible targets, and the UI asks where that dialogue should go.
reassign_to then moves every utterance recorded under the speaker's name OR
any of its aliases - utterances are frequently stored under an alias - and
keeps the old name as an alias on the target so existing references still
resolve. A speaker with no dialogue is simply removed.

Also removes the speaker's own reference audio (never the shared narrator or
general ref) and its voice_status approval entry, so a speaker re-created
later cannot inherit an approval nobody gave for it.

The button is no longer hx-delete: htmx does not swap a 409, and deleting
renumbers every speaker after it, so the handler re-renders the whole list.

Verified live on Deal Hunter, reversibly: create throwaway -> delete (200,
gone) -> recreate with an utterance attached -> delete (409, count 1, 61
targets offered) -> reassign_to=Warbots (200, utterance restored, name kept
as alias) -> compared against a pre-test backup, speaker count and utterance
count identical.

10 new tests. Suite: 708 passed, 8 pre-existing.

tests/test_delete_speaker.py

@@ -0,0 +1,204 @@
+"""Tests for speaker deletion.
+
+The speakers page has offered a Delete button for a long time. The endpoint
+it called, ``DELETE /api/speakers/{index}``, did not exist: htmx sent the
+request and the server answered ``405 Method Not Allowed``, so the button
+silently did nothing.
+
+Deleting a speaker who still has dialogue is also destructive in a way that
+is easy to miss - the utterances keep a ``speaker_name`` that no longer
+resolves - so the endpoint refuses unless the caller says where that dialogue
+should go.
+"""
+from __future__ import annotations
+
+import json
+import sqlite3
+from collections.abc import Iterator
+from pathlib import Path
+from typing import Any
+
+import pytest
+from fastapi.testclient import TestClient
+
+SPEAKERS = [
+    {"primary_name": "Alpha", "aliases": ["A-one"], "vocal_traits": ["calm"]},
+    {"primary_name": "Beta", "aliases": [], "vocal_traits": ["deep"]},
+    {"primary_name": "Silent", "aliases": [], "vocal_traits": []},
+]
+
+
+@pytest.fixture()
+def data_dir(tmp_path: Path) -> Iterator[Path]:
+    """A throwaway project, wired in as the active one."""
+    from umbraaudio.config import AppConfig
+
+    d = tmp_path / "proj" / ".umbra"
+    d.mkdir(parents=True)
+    (d / "speakers.json").write_text(json.dumps({"speakers": SPEAKERS}))
+
+    # Use the app's own schema: _get_db runs SCHEMA_SQL on connect, and a
+    # hand-rolled minimal table fails against it.
+    from umbraaudio.web.routes import SCHEMA_SQL
+
+    conn = sqlite3.connect(str(d / "project.db"))
+    conn.executescript(SCHEMA_SQL)
+    conn.execute(
+        "INSERT INTO utterances (chapter_id, utterance_index, speaker_name, text, type) "
+        "VALUES (1, 0, 'Beta', 'hello', 'dialogue')"
+    )
+    conn.execute(
+        "INSERT INTO utterances (chapter_id, utterance_index, speaker_name, text, type) "
+        "VALUES (1, 1, 'A-one', 'via alias', 'dialogue')"
+    )
+    conn.commit()
+    conn.close()
+
+    cfg = AppConfig()
+    cfg.project_dir = tmp_path / "proj"
+    cfg.data_dir = d
+
+    import umbraaudio.config as config_mod
+
+    original = config_mod.load_config
+    config_mod.load_config = lambda _slug=None: cfg  # type: ignore[assignment]
+    try:
+        yield d
+    finally:
+        config_mod.load_config = original  # type: ignore[assignment]
+
+
+@pytest.fixture()
+def client(data_dir: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[TestClient]:
+    from umbraaudio.config import AppConfig
+    from umbraaudio.web import create_app
+
+    cfg = AppConfig()
+    cfg.project_dir = data_dir.parent
+    cfg.data_dir = data_dir
+
+    # The app is gated by fail-closed auth middleware, so an unauthenticated
+    # request is redirected to /login and TestClient follows it - which would
+    # turn every assertion into a check against the login page. Mint a real
+    # session with a pinned secret so the tests neither touch the production
+    # .umbra/session_secret nor need the real password.
+    import umbraaudio.auth as auth_mod
+
+    monkeypatch.setattr(auth_mod, "_session_secret", lambda: b"k" * 32)
+    token = auth_mod.issue_session(auth_mod.USERNAME)
+
+    # create_app caches the app in a module-level singleton, so it must be
+    # cleared between tests or the second test would reuse the first test's
+    # config - and therefore the first test's temporary directory.
+    import umbraaudio.web as web_mod
+
+    web_mod._app = None
+    try:
+        c = TestClient(create_app(cfg), follow_redirects=False)
+        c.cookies.set("active_project", "testproj")
+        c.cookies.set(auth_mod.COOKIE_NAME, token)
+        yield c
+    finally:
+        web_mod._app = None
+
+
+def _spk(data_dir: Path) -> list[dict[str, Any]]:
+    return json.loads((data_dir / "speakers.json").read_text())["speakers"]
+
+
+def _names(data_dir: Path) -> list[str]:
+    return [s["primary_name"] for s in _spk(data_dir)]
+
+
+def _rows(data_dir: Path) -> dict[str, str]:
+    conn = sqlite3.connect(str(data_dir / "project.db"))
+    rows = dict(conn.execute("SELECT text, speaker_name FROM utterances"))
+    conn.close()
+    return rows
+
+
+def test_delete_speaker_without_dialogue_succeeds(
+    client: TestClient, data_dir: Path
+) -> None:
+    idx = _names(data_dir).index("Silent")
+    assert client.delete(f"/api/speakers/{idx}").status_code == 200
+    assert "Silent" not in _names(data_dir)
+
+
+def test_delete_speaker_with_dialogue_is_refused(
+    client: TestClient, data_dir: Path
+) -> None:
+    """Otherwise the utterances keep a name that no longer resolves."""
+    idx = _names(data_dir).index("Beta")
+    r = client.delete(f"/api/speakers/{idx}")
+    assert r.status_code == 409
+    body = r.json()
+    assert body["utterance_count"] == 1
+    assert "Beta" in body["error"]
+    assert "Beta" in _names(data_dir), "must not be removed"
+
+
+def test_refusal_offers_the_other_speakers_as_targets(
+    client: TestClient, data_dir: Path
+) -> None:
+    idx = _names(data_dir).index("Beta")
+    options = client.delete(f"/api/speakers/{idx}").json()["options"]
+    assert "Alpha" in options
+    assert "Beta" not in options, "a speaker cannot be reassigned to itself"
+
+
+def test_reassigning_moves_the_dialogue(client: TestClient, data_dir: Path) -> None:
+    idx = _names(data_dir).index("Beta")
+    assert client.delete(f"/api/speakers/{idx}?reassign_to=Alpha").status_code == 200
+    assert _rows(data_dir)["hello"] == "Alpha"
+    assert "Beta" not in _names(data_dir)
+
+
+def test_reassignment_keeps_the_old_name_as_an_alias(
+    client: TestClient, data_dir: Path
+) -> None:
+    """So anything still referring to the deleted name keeps resolving."""
+    idx = _names(data_dir).index("Beta")
+    client.delete(f"/api/speakers/{idx}?reassign_to=Alpha")
+    alpha = next(s for s in _spk(data_dir) if s["primary_name"] == "Alpha")
+    assert "Beta" in alpha["aliases"]
+
+
+def test_dialogue_stored_under_an_alias_also_counts(
+    client: TestClient, data_dir: Path
+) -> None:
+    """Utterances are frequently stored under an alias, not the primary name."""
+    idx = _names(data_dir).index("Alpha")
+    r = client.delete(f"/api/speakers/{idx}")
+    assert r.status_code == 409
+    assert r.json()["utterance_count"] == 1
+
+
+def test_reassignment_target_matches_case_insensitively(
+    client: TestClient, data_dir: Path
+) -> None:
+    idx = _names(data_dir).index("Alpha")
+    assert client.delete(f"/api/speakers/{idx}?reassign_to=beta").status_code == 200
+    assert _rows(data_dir)["via alias"] == "Beta"
+    assert "Alpha" not in _names(data_dir)
+
+
+def test_unknown_reassignment_target_is_rejected(
+    client: TestClient, data_dir: Path
+) -> None:
+    idx = _names(data_dir).index("Beta")
+    assert client.delete(f"/api/speakers/{idx}?reassign_to=Nobody").status_code == 400
+    assert "Beta" in _names(data_dir), "must not delete"
+
+
+def test_out_of_range_index_is_rejected(client: TestClient) -> None:
+    assert client.delete("/api/speakers/999").status_code == 400
+
+
+def test_deleting_leaves_other_speakers_untouched(
+    client: TestClient, data_dir: Path
+) -> None:
+    idx = _names(data_dir).index("Silent")
+    client.delete(f"/api/speakers/{idx}")
+    alpha = next(s for s in _spk(data_dir) if s["primary_name"] == "Alpha")
+    assert alpha["aliases"] == ["A-one"], "unrelated aliases must be untouched"

umbraaudio/web/routes.py

@@ -2419,6 +2419,126 @@ async def merge_speakers(request: Request, source_index: int, target_index: int)
     )
 
 
+@router.delete("/api/speakers/{index}")
+async def delete_speaker(request: Request, index: int, reassign_to: str = ""):
+    """Delete a speaker. The UI has offered this since before the endpoint existed.
+
+    Deleting a speaker who still has dialogue would leave every one of their
+    utterances pointing at a name that no longer resolves - the orphaned-
+    utterance condition this project already has a review page for. So a
+    speaker with dialogue is refused unless the caller says where that
+    dialogue should go; ``reassign_to`` re-points it, exactly as a merge does.
+    A speaker with no dialogue is simply removed.
+    """
+    speakers = _load_speakers(request)
+    if index < 0 or index >= len(speakers):
+        return JSONResponse({"error": "invalid index"}, status_code=400)
+
+    speaker = speakers[index]
+    name = speaker.get("primary_name", "")
+    old_names = [n for n in ([name] + speaker.get("aliases", [])) if n]
+
+    db = _get_db(request)
+    placeholders = ",".join("?" for _ in old_names) or "''"
+    count = db.execute(
+        f"SELECT COUNT(*) FROM utterances WHERE speaker_name IN ({placeholders})",
+        old_names,
+    ).fetchone()[0]
+
+    if count and not reassign_to:
+        others = [
+            s.get("primary_name", "")
+            for i, s in enumerate(speakers)
+            if i != index and s.get("primary_name")
+        ]
+        return JSONResponse(
+            {
+                "error": (
+                    f"{name} has {count} utterance(s). Choose which speaker "
+                    "should speak them instead."
+                ),
+                "utterance_count": count,
+                "speaker_name": name,
+                "options": others,
+            },
+            status_code=409,
+        )
+
+    if reassign_to:
+        target = next(
+            (
+                s
+                for s in speakers
+                if s.get("primary_name", "").strip().casefold()
+                == reassign_to.strip().casefold()
+            ),
+            None,
+        )
+        if target is None:
+            return JSONResponse(
+                {"error": f"no speaker named {reassign_to!r} to reassign to"},
+                status_code=400,
+            )
+        target_name = target.get("primary_name", "")
+        if name and name not in target.get("aliases", []):
+            target.setdefault("aliases", []).append(name)
+        moved = 0
+        for old_name in old_names:
+            cur = db.execute(
+                "UPDATE utterances SET speaker_name = ? WHERE speaker_name = ?",
+                (target_name, old_name),
+            )
+            moved += cur.rowcount or 0
+        db.commit()
+    else:
+        target_name = ""
+        moved = 0
+
+    # Remove this speaker's own reference audio, but never a shared one.
+    removed_audio: list[str] = []
+    config = _get_active_config(request)
+    voices_dir = config.data_dir / "voices"
+    slug = name.strip().lower().replace(" ", "_")
+    if slug and slug not in ("narrator", "general"):
+        for candidate in (slug + "_ref.wav", slug + ".wav"):
+            ref = voices_dir / candidate
+            if ref.exists():
+                try:
+                    ref.unlink()
+                    removed_audio.append(candidate)
+                except OSError as e:  # never fail the delete over a stale file
+                    logger.warning("Could not remove %s: %s", ref, e)
+
+    del speakers[index]
+    _save_speakers(request, speakers)
+
+    # Drop the approval entry too. Leaving a stale "approved" behind would let a
+    # speaker re-created later inherit an approval nobody gave for it.
+    if name:
+        ledger = config.data_dir / "voice_status.json"
+        try:
+            if ledger.exists():
+                data = json.loads(ledger.read_text())
+                if isinstance(data, dict) and data.pop(name, None) is not None:
+                    ledger.write_text(json.dumps(data, indent=2))
+        except (OSError, ValueError) as e:
+            logger.warning("Could not update voice_status ledger: %s", e)
+
+    speakers = _load_speakers(request)
+    return templates.TemplateResponse(
+        request,
+        "_speakers_list.html",
+        {
+            "request": request,
+            "speakers": speakers,
+            "deleted": name,
+            "reassigned_to": target_name,
+            "utterances_moved": moved,
+            "removed_audio": removed_audio,
+        },
+    )
+
+
 @router.get("/orphaned-utterances", response_class=HTMLResponse)
 async def orphaned_utterances_page(request: Request):
     db = _get_db(request)

umbraaudio/web/templates/_speaker_row.html

@@ -140,13 +140,15 @@
                 onclick="toggleEditSpeaker({{ speaker_idx }})">
             Edit
         </button>
-        <button class="btn btn-reject" style="font-size:0.8rem; padding:0.3rem 0.6rem"
-                hx-delete="/api/speakers/{{ speaker_idx }}"
-                hx-target="#speaker-row-{{ speaker_idx }}"
-                hx-swap="outerHTML"
-                hx-confirm="Delete {{ speaker.primary_name }}?">
-            Delete
-        </button>
+          {# Not hx-delete: htmx will not swap a 409, and this has to be
+             able to answer "this speaker has 12 utterances - where do they
+             go?". The handler also re-renders the whole list, because
+             deleting renumbers every speaker after it. #}
+          <button class="btn btn-reject" style="font-size:0.8rem; padding:0.3rem 0.6rem"
+                  onclick="deleteSpeaker({{ speaker_idx }}, '{{ speaker.primary_name|e }}')">
+              Delete
+          </button>
+          <div id="delete-prompt-{{ speaker_idx }}" style="display:none; margin-top:0.5rem; padding:0.5rem; background:#3f1d1d; border-radius:6px"></div>
         <div id="merge-speaker-{{ speaker_idx }}" style="display:none; margin-top:0.5rem; border-top:1px solid #333; padding-top:0.5rem">
             <div style="display:flex; gap:0.5rem; align-items:center; margin-bottom:0.5rem">
                 <label style="color:#94a3b8; font-size:0.8rem">Merge into:</label>

umbraaudio/web/templates/speakers.html

@@ -108,6 +108,80 @@ function notify(message, isError) {
     setTimeout(function () { el.remove(); }, 5000);
 }
 
+// Delete a speaker.
+//
+// A plain hx-delete could not do this job: htmx does not swap a 409, and the
+// server refuses to delete a speaker who still has dialogue unless told where
+// that dialogue goes. Otherwise the utterances keep a speaker_name that no
+// longer resolves - an orphaned utterance.
+//
+// The whole list is re-rendered on success because deleting renumbers every
+// speaker after the removed one; swapping only the row would leave every
+// later row pointing at the wrong speaker.
+function deleteSpeaker(idx, name) {
+    if (!confirm('Delete ' + name + '?')) return;
+    requestDelete(idx, '');
+}
+
+function requestDelete(idx, reassignTo) {
+    var url = '/api/speakers/' + idx +
+        (reassignTo ? '?reassign_to=' + encodeURIComponent(reassignTo) : '');
+    fetch(url, {method: 'DELETE', headers: {'HX-Request': 'true'}})
+        .then(function (r) {
+            return r.json().catch(function () { return {}; })
+                .then(function (d) { return {status: r.status, data: d}; });
+        })
+        .then(function (res) {
+            if (res.status === 409) {
+                showReassignPrompt(idx, nameOf(res.data), res.data);
+                return;
+            }
+            if (res.status !== 200) {
+                notify('Delete failed: ' + (res.data.error || res.status), true);
+                return;
+            }
+            htmx.ajax('GET', '/speakers/partial', {
+                target: '#speakers-list', swap: 'innerHTML'
+            });
+            var msg = 'Deleted ' + nameOf(res.data) + '.';
+            notify(msg);
+            initSpeakerNames();
+        })
+        .catch(function (e) {
+            notify('Delete failed: ' + e, true);
+        });
+}
+
+function nameOf(data) {
+    return (data && data.speaker_name) || 'speaker';
+}
+
+function showReassignPrompt(idx, name, data) {
+    var box = document.getElementById('delete-prompt-' + idx);
+    if (!box) {
+        notify('Cannot delete: ' + (data.error || 'has utterances'), true);
+        return;
+    }
+    var opts = (data.options || []).map(function (n) {
+        return '<option value="' + n.replace(/"/g, '&quot;') + '">' + n + '</option>';
+    }).join('');
+    box.innerHTML =
+        '<div style="color:#fecaca; font-size:0.8rem; margin-bottom:0.4rem">' +
+        (data.error || 'This speaker still has dialogue.') + '</div>' +
+        '<select id="delete-target-' + idx + '" ' +
+        'style="width:100%; padding:0.3rem; background:#1a1a2e; color:#e0e0e0; ' +
+        'border:1px solid #555; border-radius:3px; margin-bottom:0.4rem">' +
+        opts + '</select>' +
+        '<div style="display:flex; gap:0.4rem">' +
+        '<button class="btn btn-reject" style="font-size:0.75rem" ' +
+        'onclick="requestDelete(' + idx + ', document.getElementById(\'delete-target-' +
+        idx + '\').value)">Reassign and delete</button>' +
+        '<button class="btn" style="font-size:0.75rem" ' +
+        'onclick="document.getElementById(\'delete-prompt-' + idx + '\').style.display=\'none\'">' +
+        'Cancel</button></div>';
+    box.style.display = 'block';
+}
+
 function queueVoiceRegenOnSave(idx) {
     var row = document.getElementById('speaker-row-' + idx);
     var label = row ? row.querySelector('strong') : null;