fix(dub): clone references live in their own job dir; deletes spare shared files (#1331) (#1381)

Root cause fix for #1331, covered from both directions.

Extraction half: on a content-hash cache hit the new job's vocals_path points into the old job's directory (cache working as designed), and both extraction call sites — per-speaker and the default per-segment — then wrote the new job's clone references into that older directory too. Deleting the older history entry orphaned them: every single-segment regen silently rendered in the default voice, and a full re-dub "fixed" it only because prep re-extracts. Clones now land in the current job's own dir; the AST test sweeps every extraction call so a third copy inherits the rule.

Deletion half: existing users' jobs already carry cross-directory references, and vocals are shared by design — so deleting a dub now checks whether any other saved dub still references files in its directory. If so, the history row is removed (the entry disappears as asked) but the directory is kept, with the holder logged. Clear-all untouched. 13 tests across both halves.

CHANGELOG.md

@@ -30,6 +30,8 @@ The bundled TTS model package (`pyproject.toml`) is versioned independently.
 ### Fixed
 
 - Every subprocess TTS engine would have turned a stereo render into noise: the mono downmix always averaged axis 0, which is time rather than channels for channels-last audio. Unreachable today since every engine returns mono, fixed in all five before it isn't. (#1328)
+- Dubbing the same video twice no longer ties the second job's cloned voices to the first job's files — deleting the older dub from history was silently turning the newer one's single-segment regens into a default voice. (#1331)
+- ...and deleting a dub whose files an existing saved dub still renders from now keeps those files on disk (the history entry still disappears) — protecting dubs created before this fix, whose references already cross directories. (#1331)
 - An unclean previous shutdown is no longer announced as a crash: the notice says what it actually knows, names the benign causes (sleep, force-quit, a stopped VM), and the one-click bug report is only offered when there is evidence to put in it — an empty report helps nobody. (#1375)
 - A first-use generate no longer fails at 300s while its model is still downloading: the download's own progress heartbeats now extend the generation budget (bounded), so a slow connection isn't reported as too-slow hardware. A job that goes silent still dies at the original deadline. (#1367)
 - A generation that hits its time limit now says so, instead of "an error OmniVoice doesn't recognize" followed by an empty `TimeoutError:`. It names the likely causes and the setting that raises the limit. (#1368)

backend/api/routers/dub_core.py

@@ -252,13 +252,28 @@ def delete_single_dub_history(history_id: str):
         with db_conn() as conn:
             conn.execute("DELETE FROM dub_history WHERE id=?", (history_id,))
 
+    # #1331 (deletion half): the content-hash cache points newer jobs' paths
+    # (vocals, and pre-fix clone refs) into this dir. Check BEFORE the row is
+    # deleted — the scan reads dub_history, and after _delete_row this row's
+    # neighbours are all that's left to consult either way.
+    holders = dub_pipeline.job_dir_referenced_by_others(history_id)
+
     # Atomic with the evict — see purge_jobs (#1252 review).
     dub_pipeline.purge_jobs([history_id], delete_rows=_delete_row)
     safe = _safe_job_dir(history_id)
-    if safe and os.path.isdir(safe):
+    if holders:
+        # Keep the directory: another saved dub still renders from files in
+        # it. Disk is the cheap thing here; a job that silently loses its
+        # cloned voice on every regen is not. The row is gone, so the entry
+        # disappears from history either way.
+        logger.info(
+            "dub delete %s: history row removed but directory kept — still "
+            "referenced by job(s) %s (#1331)", history_id, ", ".join(holders),
+        )
+    elif safe and os.path.isdir(safe):
         shutil.rmtree(safe, ignore_errors=True)
     event_bus.emit("dub_history", {"action": "deleted", "id": history_id})
-    return {"deleted": True}
+    return {"deleted": True, "dir_kept_for": holders}
 
 @router.post("/preview/upload")
 async def preview_upload(video: UploadFile = File(...)):
@@ -1265,10 +1280,18 @@ async def dub_transcribe_stream(
                     "source": "speaker_clone",
                 })
             else:
+                # Clones are written into THIS job's dir, never alongside the
+                # vocals (#1331): on a content-hash cache hit vocals_path
+                # points into an OLDER job's dir, so dirname(vocals) wrote the
+                # new job's clone refs into a directory the user can delete by
+                # removing that older history entry — after which every
+                # single-segment regen silently rendered in the default voice.
+                _clone_dir = _safe_job_dir(job_id) or os.path.dirname(vocals_for_clone)
+                os.makedirs(_clone_dir, exist_ok=True)
                 fut_clones = loop.run_in_executor(
                     _cpu_pool, lambda: extract_speaker_clones(
                         vocals_for_clone, final_segs,
-                        os.path.dirname(vocals_for_clone),
+                        _clone_dir,
                         labels_source=labels_source,
                     ),
                 )
@@ -1307,10 +1330,17 @@ async def dub_transcribe_stream(
                 try:
                     from services.speaker_clone import extract_segment_refs
                     seg_ids_for_clone = [s.get("id", i) for i, s in enumerate(final_segs)]
+                    # Same #1331 rule as the per-speaker extraction above, and
+                    # this is the DEFAULT path: per-segment references must
+                    # live in THIS job's dir, or a cache-hit job's clips die
+                    # with the older job they were written next to (both
+                    # reviewers, on the first version of this fix).
+                    _seg_clone_dir = _safe_job_dir(job_id) or os.path.dirname(vocals_for_clone)
+                    os.makedirs(_seg_clone_dir, exist_ok=True)
                     seg_clones = await loop.run_in_executor(
                         _cpu_pool, lambda: extract_segment_refs(
                             vocals_for_clone, final_segs,
-                            os.path.dirname(vocals_for_clone),
+                            _seg_clone_dir,
                             seg_ids=seg_ids_for_clone,
                         ),
                     )

backend/services/dub_pipeline.py

@@ -137,6 +137,45 @@ def safe_job_dir(job_id: str) -> Optional[str]:
     return candidate
 
 
+def job_dir_referenced_by_others(job_id: str) -> "list[str]":
+    """History ids of OTHER jobs whose persisted paths point into ``job_id``'s
+    directory (#1331, the deletion half).
+
+    The content-hash cache legitimately points a newer job's ``vocals_path``
+    (and, for jobs created before the job-scoped-clones fix, its clone
+    reference paths) into an older job's directory. Deleting that older entry
+    used to ``rmtree`` the dir regardless, silently breaking the newer job:
+    single-segment regens fell back to the default voice, stems exports lost
+    their sources. The caller uses this to keep the DIRECTORY while still
+    deleting the history row — disk is the cheap thing here; another job's
+    voice is not.
+
+    Scans persisted ``job_data`` as text for the dir prefix rather than
+    enumerating every path-bearing key: keys have grown before (vocals,
+    no_vocals, thumb, clone refs, segment refs) and a scan cannot fall behind
+    the schema.
+    """
+    target = safe_job_dir(job_id)
+    if not target:
+        return []
+    needle = target.rstrip(os.sep) + os.sep
+    # JSON-encoded job_data escapes backslashes, so match the Windows form too.
+    needle_json = needle.replace("\\", "\\\\")
+    holders: list[str] = []
+    try:
+        with db_conn() as conn:
+            rows = conn.execute(
+                "SELECT id, job_data FROM dub_history WHERE id != ?", (job_id,)
+            ).fetchall()
+    except Exception:
+        return []  # no DB, nothing persisted can reference us
+    for row in rows:
+        data = row["job_data"] or ""
+        if needle in data or needle_json in data:
+            holders.append(row["id"])
+    return holders
+
+
 def sse_event(event: str, payload) -> bytes:
     """Encode one Server-Sent Event frame. UTF-8 bytes, ready to yield."""
     return f"event: {event}\ndata: {json.dumps(payload, ensure_ascii=False)}\n\n".encode("utf-8")

tests/test_clone_refs_live_in_their_own_job_dir.py

@@ -0,0 +1,172 @@
+"""A job's clone references must live in ITS directory, not a neighbour's (#1331).
+
+Reported from Discord (in Russian): re-dubbing a single sentence loses the
+cloned voice; only a full re-dub keeps it. #1361 made the mechanism visible —
+the reference path is handed to the engine but the file is gone. This is the
+root cause for the identified class:
+
+On a content-hash cache hit (same video dubbed twice), ``find_cached_job``
+points the NEW job's ``vocals_path`` into the OLD job's directory — that is the
+cache working as designed. But ``dub_core`` then extracted the new job's clone
+references into ``os.path.dirname(vocals_path)``, i.e. **into the old job's
+directory too**. Delete that older history entry — an ordinary, sanctioned
+action that ``rmtree``s its dir — and every clone reference of the newer job
+dangles. From then on each single-segment regen silently renders in the
+default voice, and a full re-dub "fixes" it only because prep re-extracts.
+
+The fix is one argument: clones are written into the CURRENT job's own dir.
+"""
+from __future__ import annotations
+
+import ast
+import importlib
+import os
+import sys
+
+import numpy as np
+import pytest
+
+os.environ.setdefault("OMNIVOICE_MODEL", "test")
+os.environ.setdefault("OMNIVOICE_DISABLE_FILE_LOG", "1")
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "backend"))
+
+sf = pytest.importorskip("soundfile")
+
+
+def _extraction_call():
+    """The ``extract_speaker_clones(...)`` call node in dub_core, by AST —
+    a comment mentioning the right directory must not satisfy this."""
+    src = open(
+        os.path.join(os.path.dirname(__file__), "..", "backend", "api",
+                     "routers", "dub_core.py"),
+        encoding="utf-8",
+    ).read()
+    for node in ast.walk(ast.parse(src)):
+        if (isinstance(node, ast.Call)
+                and getattr(node.func, "id", getattr(node.func, "attr", None))
+                == "extract_speaker_clones"):
+            return node, src
+    raise AssertionError("extract_speaker_clones call not found in dub_core.py")
+
+
+def test_the_out_dir_is_not_derived_from_the_vocals_path():
+    """The bug, stated directly: ``os.path.dirname(vocals_for_clone)`` follows
+    the vocals into whichever job dir the cache resolved them from."""
+    call, _ = _extraction_call()
+    out_dir_arg = ast.unparse(call.args[2])
+    assert "dirname" not in out_dir_arg, (
+        f"the clone out_dir is again derived from the vocals path "
+        f"({out_dir_arg!r}) — on a cache hit that is an OLDER job's directory, "
+        f"and deleting that job orphans this one's clone references (#1331)"
+    )
+
+
+def test_the_out_dir_comes_from_the_jobs_own_id():
+    call, src = _extraction_call()
+    out_dir_arg = ast.unparse(call.args[2])
+    # The argument is a variable; its assignment must resolve via the job id.
+    assert "_clone_dir" in out_dir_arg
+    assert "_safe_job_dir(job_id)" in src.split("fut_clones")[0].rsplit(
+        "extract_speaker_clones", 2)[0][-2000:] or "_safe_job_dir(job_id)" in src, (
+        "the clone out_dir is no longer anchored to this job's own directory"
+    )
+
+
+def test_extraction_honours_an_out_dir_away_from_the_vocals(tmp_path):
+    """Functional half: vocals in one job's dir, clones requested in another's
+    — the written reference files must land in the requested dir, so deleting
+    the vocals-owning job cannot orphan them."""
+    from services.speaker_clone import extract_speaker_clones
+
+    old_job = tmp_path / "job-old"
+    new_job = tmp_path / "job-new"
+    old_job.mkdir()
+    new_job.mkdir()
+
+    sr = 16000
+    rng = np.random.default_rng(0)
+    audio = (rng.standard_normal(sr * 12) * 0.1).astype(np.float32)
+    vocals = old_job / "vocals.wav"
+    sf.write(vocals, audio, sr)
+
+    segments = [
+        {"start": 0.0, "end": 5.5, "text": "first line", "speaker_id": "Speaker 1"},
+        {"start": 6.0, "end": 11.5, "text": "second line", "speaker_id": "Speaker 1"},
+    ]
+    clones = extract_speaker_clones(str(vocals), segments, str(new_job))
+    assert clones, "no clone extracted from 11s of speech"
+    for info in clones.values():
+        ref = info["ref_audio"]
+        assert os.path.dirname(ref) == str(new_job), (
+            f"clone reference written next to the vocals ({ref}) instead of "
+            f"into the requesting job's dir"
+        )
+        assert os.path.isfile(ref)
+
+    # The point of the whole exercise: the old job's dir can now die without
+    # taking the new job's voice with it.
+    import shutil
+    shutil.rmtree(old_job)
+    for info in clones.values():
+        assert os.path.isfile(info["ref_audio"])
+
+
+def _all_extraction_calls():
+    """BOTH extraction call sites in dub_core — the per-speaker one and the
+    per-segment one (the default). The first version of this fix covered only
+    the former; both reviewers caught the latter, which is how the class
+    survives a spot fix. Sweeping every call keeps a third copy honest too."""
+    src = open(
+        os.path.join(os.path.dirname(__file__), "..", "backend", "api",
+                     "routers", "dub_core.py"),
+        encoding="utf-8",
+    ).read()
+    calls = [
+        node for node in ast.walk(ast.parse(src))
+        if isinstance(node, ast.Call)
+        and getattr(node.func, "id", getattr(node.func, "attr", None))
+        in ("extract_speaker_clones", "extract_segment_refs")
+    ]
+    assert len(calls) >= 2, "expected both extraction call sites in dub_core"
+    return calls
+
+
+def test_no_extraction_call_derives_its_out_dir_from_the_vocals():
+    for call in _all_extraction_calls():
+        out_dir_arg = ast.unparse(call.args[2])
+        assert "dirname" not in out_dir_arg, (
+            f"an extraction call writes next to the vocals again "
+            f"({ast.unparse(call.func)}: {out_dir_arg!r}) — on a cache hit "
+            f"that is an older job's directory (#1331)"
+        )
+
+
+def test_segment_refs_honour_an_out_dir_away_from_the_vocals(tmp_path):
+    """Functional half for the DEFAULT path: per-segment clips land in the
+    requesting job's dir and survive the vocals-owning job's deletion."""
+    from services.speaker_clone import extract_segment_refs
+
+    old_job = tmp_path / "job-old"
+    new_job = tmp_path / "job-new"
+    old_job.mkdir()
+    new_job.mkdir()
+
+    sr = 16000
+    rng = np.random.default_rng(1)
+    audio = (rng.standard_normal(sr * 12) * 0.1).astype(np.float32)
+    vocals = old_job / "vocals.wav"
+    sf.write(vocals, audio, sr)
+
+    segments = [
+        {"start": 0.0, "end": 5.5, "text": "first line", "speaker_id": "Speaker 1"},
+        {"start": 6.0, "end": 11.5, "text": "second line", "speaker_id": "Speaker 1"},
+    ]
+    refs = extract_segment_refs(str(vocals), segments, str(new_job), seg_ids=[0, 1])
+    assert refs, "no per-segment refs extracted from two 5.5s lines"
+
+    import shutil
+    shutil.rmtree(old_job)
+    for info in refs.values():
+        assert os.path.dirname(info["ref_audio"]) == str(new_job)
+        assert os.path.isfile(info["ref_audio"])

tests/test_deleting_a_dub_keeps_dirs_others_use.py

@@ -0,0 +1,178 @@
+"""Deleting a dub must not destroy files another saved dub still renders from.
+
+The deletion half of #1331. The content-hash cache legitimately points a newer
+job's ``vocals_path`` — and, for jobs created before the job-scoped-clones fix,
+its clone reference paths — into an older job's directory. Deleting that older
+history entry ``rmtree``'d the dir regardless: the newer job kept loading, but
+every single-segment regen silently fell back to the default voice and stems
+exports lost their sources.
+
+The guard is deliberately on the DELETE side even though new jobs no longer
+write clone refs into neighbour dirs: existing users' jobs, created before that
+fix, still carry cross-directory paths, and vocals are shared by design either
+way. The history row still goes — the entry disappears from the UI — only the
+directory survives while someone else needs it. Disk is the cheap thing here.
+"""
+from __future__ import annotations
+
+import importlib
+import json
+import os
+import sys
+
+import pytest
+
+os.environ.setdefault("OMNIVOICE_MODEL", "test")
+os.environ.setdefault("OMNIVOICE_DISABLE_FILE_LOG", "1")
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "backend"))
+
+
+@pytest.fixture
+def dp(tmp_path, monkeypatch):
+    """dub_pipeline with DUB_DIR + the history DB isolated to tmp_path."""
+    mod = importlib.import_module("services.dub_pipeline")
+    dub_dir = tmp_path / "dub"
+    dub_dir.mkdir()
+    monkeypatch.setattr(mod, "DUB_DIR", str(dub_dir))
+    monkeypatch.setattr(mod, "_DUB_DIR_REAL", os.path.realpath(str(dub_dir)))
+
+    import sqlite3
+
+    db = sqlite3.connect(":memory:", check_same_thread=False)
+    db.row_factory = sqlite3.Row
+    db.execute("CREATE TABLE dub_history (id TEXT PRIMARY KEY, job_data TEXT)")
+
+    class _Conn:
+        def __enter__(self):
+            return db
+
+        def __exit__(self, *a):
+            db.commit()
+
+    monkeypatch.setattr(mod, "db_conn", lambda: _Conn())
+    return mod, db, dub_dir
+
+
+def _add_job(db, job_id, job_data: dict):
+    db.execute(
+        "INSERT INTO dub_history (id, job_data) VALUES (?, ?)",
+        (job_id, json.dumps(job_data)),
+    )
+
+
+def test_a_dir_no_other_job_references_reports_no_holders(dp):
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    _add_job(db, "job-b", {"vocals_path": str(dub_dir / "job-b" / "vocals.wav")})
+    assert mod.job_dir_referenced_by_others("job-a") == []
+
+
+def test_a_cache_sharing_job_is_detected_as_a_holder(dp):
+    """The reported shape: job B's vocals live in job A's directory."""
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    _add_job(db, "job-b", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    assert mod.job_dir_referenced_by_others("job-a") == ["job-b"]
+
+
+def test_pre_fix_clone_refs_are_detected_too(dp):
+    """Jobs created before the job-scoped-clones fix carry clone reference
+    paths — not vocals — into the neighbour dir. The scan is textual over the
+    whole job_data precisely so it does not care which key holds the path."""
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    _add_job(db, "job-b", {
+        "vocals_path": str(dub_dir / "job-b" / "vocals.wav"),
+        "segment_clones": {
+            "7": {"ref_audio": str(dub_dir / "job-a" / "seg_7.wav")},
+        },
+    })
+    assert mod.job_dir_referenced_by_others("job-a") == ["job-b"]
+
+
+def test_the_job_being_deleted_does_not_count_as_its_own_holder(dp):
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    assert mod.job_dir_referenced_by_others("job-a") == []
+
+
+def test_a_similar_prefix_is_not_a_false_positive(dp):
+    """job-a2's own dir starts with the string "job-a" — the needle carries a
+    trailing separator so prefix-sharing ids cannot collide."""
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": str(dub_dir / "job-a" / "vocals.wav")})
+    _add_job(db, "job-a2", {"vocals_path": str(dub_dir / "job-a2" / "vocals.wav")})
+    assert mod.job_dir_referenced_by_others("job-a") == []
+
+
+def test_windows_style_json_escaped_paths_are_detected(dp):
+    """job_data is JSON, so Windows separators are stored escaped
+    (``C:\\\\...``). The scan must match that spelling as well, or the guard
+    silently never fires on the platform with the most reports."""
+    mod, db, dub_dir = dp
+    (dub_dir / "job-a").mkdir()
+    _add_job(db, "job-a", {"vocals_path": "irrelevant"})
+    # Simulate a Windows job_data blob referencing job-a's dir with
+    # backslash separators, JSON-escaped on disk.
+    win_path = str(dub_dir / "job-a").replace("/", "\\") + "\\vocals.wav"
+    db.execute(
+        "INSERT INTO dub_history (id, job_data) VALUES (?, ?)",
+        ("job-b", json.dumps({"vocals_path": win_path})),
+    )
+    # safe_job_dir uses the real platform separator, so the full scan can't be
+    # driven cross-platform from a posix test host — pin the storage-format
+    # premise the needle_json branch exists for: JSON escapes backslashes.
+    raw = db.execute("SELECT job_data FROM dub_history WHERE id='job-b'").fetchone()[0]
+    assert (str(dub_dir / "job-a").replace("/", "\\") + "\\").replace("\\", "\\\\") in raw
+
+
+def test_the_delete_endpoint_keeps_a_referenced_dir(dp, monkeypatch):
+    """End-to-end at the router level: the row goes, the dir stays, and the
+    response says for whom."""
+    mod, db, dub_dir = dp
+    core = importlib.import_module("api.routers.dub_core")
+
+    job_a = dub_dir / "job-a"
+    job_a.mkdir(exist_ok=True)
+    (job_a / "vocals.wav").write_bytes(b"RIFF")
+    _add_job(db, "job-a", {"vocals_path": str(job_a / "vocals.wav")})
+    _add_job(db, "job-b", {"vocals_path": str(job_a / "vocals.wav")})
+
+    monkeypatch.setattr(core, "_safe_job_dir", mod.safe_job_dir)
+    monkeypatch.setattr(core, "db_conn", mod.db_conn)  # endpoint's own import
+    monkeypatch.setattr(core.dub_pipeline, "purge_jobs",
+                        lambda ids, delete_rows, **k: delete_rows())
+
+    result = core.delete_single_dub_history("job-a")
+    assert result == {"deleted": True, "dir_kept_for": ["job-b"]}
+    assert job_a.is_dir(), "the directory job-b renders from was deleted"
+    assert (job_a / "vocals.wav").is_file()
+    row = db.execute("SELECT id FROM dub_history WHERE id='job-a'").fetchone()
+    assert row is None, "the history row must still be removed"
+
+
+def test_the_delete_endpoint_still_removes_an_unreferenced_dir(dp, monkeypatch):
+    """The guard must not turn every delete into a keep — an unshared job's
+    directory still dies with its row."""
+    mod, db, dub_dir = dp
+    core = importlib.import_module("api.routers.dub_core")
+
+    job_c = dub_dir / "job-c"
+    job_c.mkdir()
+    (job_c / "vocals.wav").write_bytes(b"RIFF")
+    _add_job(db, "job-c", {"vocals_path": str(job_c / "vocals.wav")})
+
+    monkeypatch.setattr(core, "_safe_job_dir", mod.safe_job_dir)
+    monkeypatch.setattr(core, "db_conn", mod.db_conn)
+    monkeypatch.setattr(core.dub_pipeline, "purge_jobs",
+                        lambda ids, delete_rows, **k: delete_rows())
+
+    result = core.delete_single_dub_history("job-c")
+    assert result == {"deleted": True, "dir_kept_for": []}
+    assert not job_c.exists(), "an unreferenced dir must still be cleaned up"