diff --git a/CHANGELOG.md b/CHANGELOG.md index befdfc4a..903758a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -322,7 +322,10 @@ across dub, generate, and design (a corrupt-binary failure no longer poses as and retries the load automatically. Offline mode (`HF_HUB_OFFLINE`) is respected — repair never makes a network call the user opted out of — and if the re-fetch still can't fix it, the actionable delete-and-reinstall message - is preserved as the fallback. (#581) + is preserved as the fallback. (#581) The repair now also **retries** the + re-fetch (3 attempts, resuming each time) so a single transient blip — the very + thing that interrupts a download in the first place — doesn't bounce you back + to a manual reinstall; tune with `OMNIVOICE_MODEL_REPAIR_RETRIES`. (#739) - **Dubbing a YouTube URL no longer dies on a transient "Broken pipe."** Pasting a video link could fail outright with `download: Unable to download video: [Errno 32] Broken pipe` — a broken pipe raised while the write side of diff --git a/backend/services/model_manager.py b/backend/services/model_manager.py index b9992a52..f57eab1f 100644 --- a/backend/services/model_manager.py +++ b/backend/services/model_manager.py @@ -578,7 +578,6 @@ def _repair_model_cache(checkpoint: str) -> bool: except Exception as imp_err: # pragma: no cover - huggingface_hub is a hard dep logger.warning("Cannot import snapshot_download to repair cache: %s", imp_err) return False - logger.info("Auto-repairing incomplete model cache for %s …", checkpoint) dl_kwargs: dict = {"repo_id": checkpoint} endpoint = os.environ.get("HF_ENDPOINT") if endpoint: @@ -586,22 +585,50 @@ def _repair_model_cache(checkpoint: str) -> bool: if os.name == "nt": # Match the install path (download.py): avoid symlinks on Windows. dl_kwargs["local_dir_use_symlinks"] = False - try: - snapshot_download(**dl_kwargs) - except TypeError: - # Older/newer huggingface_hub may not accept local_dir_use_symlinks - # on a cache-only call — retry without the optional knob. - dl_kwargs.pop("local_dir_use_symlinks", None) + + def _attempt() -> None: + """One snapshot_download, tolerating an hf_hub that rejects the optional + symlink knob. Lets real failures (network, gated repo, disk) propagate.""" try: snapshot_download(**dl_kwargs) + except TypeError: + # Older/newer huggingface_hub may not accept local_dir_use_symlinks + # on a cache-only call — retry without the optional knob. + dl_kwargs.pop("local_dir_use_symlinks", None) + snapshot_download(**dl_kwargs) + + # Bounded retries (#739): an incomplete cache *is* an interrupted download, so + # a single transient blip mid-repair shouldn't drop the user back to a manual + # delete-and-reinstall. snapshot_download resumes between attempts (present, + # correctly-sized blobs are skipped by hash), so each retry continues where + # the last left off — cheap and idempotent. Counts/backoff are env-tunable + # for restricted networks and kept fast (backoff=0) in tests. + try: + retries = max(1, int(os.environ.get("OMNIVOICE_MODEL_REPAIR_RETRIES", "3"))) + except ValueError: + retries = 3 + try: + backoff = max(0.0, float(os.environ.get("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "2"))) + except ValueError: + backoff = 2.0 + + logger.info( + "Auto-repairing incomplete model cache for %s (up to %d attempt(s)) …", + checkpoint, retries, + ) + for attempt in range(1, retries + 1): + try: + _attempt() + logger.info("Auto-repair of %s completed; retrying model load.", checkpoint) + return True except Exception as e: - logger.warning("Auto-repair of %s failed: %s", checkpoint, e) - return False - except Exception as e: - logger.warning("Auto-repair of %s failed: %s", checkpoint, e) - return False - logger.info("Auto-repair of %s completed; retrying model load.", checkpoint) - return True + logger.warning( + "Auto-repair of %s attempt %d/%d failed: %s", + checkpoint, attempt, retries, e, + ) + if attempt < retries and backoff: + time.sleep(backoff * attempt) + return False _DEFAULT_OMNIVOICE_CHECKPOINT = "k2-fsa/OmniVoice" diff --git a/tests/test_model_cache_repair.py b/tests/test_model_cache_repair.py index d7853c44..05378dba 100644 --- a/tests/test_model_cache_repair.py +++ b/tests/test_model_cache_repair.py @@ -142,8 +142,52 @@ def test_repair_returns_false_when_download_fails(model_manager, monkeypatch): """A failed re-fetch (no network, gated repo) returns False, never raises.""" import huggingface_hub + calls = [] + def boom(**kwargs): + calls.append(kwargs) raise OSError("network down") monkeypatch.setattr(huggingface_hub, "snapshot_download", boom) + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0") # no real sleeps + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "3") assert model_manager._repair_model_cache("test/checkpoint") is False + # #739: a transient failure must be retried, not given up on after one try. + assert len(calls) == 3 + + +def test_repair_retries_then_succeeds(model_manager, monkeypatch): + """#739: a flaky connection that drops twice then completes must self-heal — + the repair retries snapshot_download and returns True, so the user is never + sent to a manual delete-and-reinstall for a transient blip.""" + import huggingface_hub + + attempts = {"n": 0} + + def flaky(**kwargs): + attempts["n"] += 1 + if attempts["n"] < 3: + raise OSError("connection reset") + return "/cache/test/checkpoint" + + monkeypatch.setattr(huggingface_hub, "snapshot_download", flaky) + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0") + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "3") + assert model_manager._repair_model_cache("test/checkpoint") is True + assert attempts["n"] == 3 + + +def test_repair_retries_are_env_tunable(model_manager, monkeypatch): + """A restricted network can lower/raise the attempt count; a single attempt + must still work (no off-by-one that skips the only try).""" + import huggingface_hub + + calls = [] + monkeypatch.setattr( + huggingface_hub, "snapshot_download", + lambda **k: calls.append(k) or (_ for _ in ()).throw(OSError("down")), + ) + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0") + monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "1") + assert model_manager._repair_model_cache("test/checkpoint") is False + assert len(calls) == 1